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) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 0A8FB62148 for ; Fri, 11 Feb 2022 09:34:02 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id EB41925ADE for ; Fri, 11 Feb 2022 09:33:31 +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) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 0C85725AD5 for ; Fri, 11 Feb 2022 09:33:31 +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 D707946DE7 for ; Fri, 11 Feb 2022 09:33:30 +0100 (CET) Date: Fri, 11 Feb 2022 09:33:23 +0100 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Fabian Ebner , pve-devel@lists.proxmox.com References: <20220209130750.902245-1-f.gruenbichler@proxmox.com> <20220209130750.902245-3-f.gruenbichler@proxmox.com> <32b79391-62b6-50a6-7238-9e2f8a630dcb@proxmox.com> In-Reply-To: <32b79391-62b6-50a6-7238-9e2f8a630dcb@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.15.0 (https://github.com/astroidmail/astroid) Message-Id: <1644567782.rcla1kth6p.astroid@nora.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL 0.192 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 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 - 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: Fri, 11 Feb 2022 08:34:02 -0000 On February 10, 2022 12:52 pm, Fabian Ebner wrote: > Am 09.02.22 um 14:07 schrieb Fabian Gr=C3=BCnbichler: >> re-using helpers that already exist for qemu-server. this is a >> pre-requisite for extending remote migration support to containers. >>=20 >> Signed-off-by: Fabian Gr=C3=BCnbichler >> --- >>=20 >> Notes: >> new in v5, no GUI yet until possible wrinkles are ironed out >> =20 >> requires pve-common/pve-guest-common with changes from this series >>=20 >> src/PVE/API2/LXC.pm | 32 ++++++++++++++++++++++ >> src/PVE/LXC/Migrate.pm | 62 ++++++++++++++++++++++++++++++++---------- >> 2 files changed, 79 insertions(+), 15 deletions(-) >>=20 >> 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 { >> } >> } >> =20 >> +my $check_storage_access_migrate =3D sub { >> + my ($rpcenv, $authuser, $storecfg, $storage, $node) =3D @_; >> + >> + PVE::Storage::storage_check_enabled($storecfg, $storage, $node); >> + >> + $rpcenv->check($authuser, "/storage/$storage", ['Datastore.Allocate= Space']); >> + >> + my $scfg =3D PVE::Storage::storage_config($storecfg, $storage); >> + die "storage '$storage' does not support CT rootdirs\n" >> + if !$scfg->{content}->{rootdir}; >> +}; >> + >> __PACKAGE__->register_method ({ >> subclass =3D> "PVE::API2::LXC::Config", >> path =3D> '{vmid}/config', >> @@ -1091,6 +1103,7 @@ __PACKAGE__->register_method({ >> description =3D> "Target node.", >> completion =3D> \&PVE::Cluster::complete_migration_target, >> }), >> + 'target-storage' =3D> get_standard_option('pve-targetstorage'), >=20 > 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=20 (same for qemu-server's remote migration API with its multiple=20 target-foo parameters ;)), and would like to phase out the=20 'targetstorage' in the regular VM migration via an alias.. >=20 >> online =3D> { >> type =3D> 'boolean', >> description =3D> "Use online/live migration.", >> @@ -1149,6 +1162,25 @@ __PACKAGE__->register_method({ >> if !$param->{online} && !$param->{restart}; >> } >> =20 >> + if (my $targetstorage =3D delete $param->{'target-storage'}) { >> + my $storecfg =3D PVE::Storage::config(); >> + my $storagemap =3D eval { PVE::JSONSchema::parse_idmap($targetstor= age, 'pve-storage-id') }; >> + raise_param_exc({ targetstorage =3D> "failed to parse storage map:= $@" }) >=20 > Should be 'target-storage' if we go with that. ack. >=20 >> + 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, $targe= t_sid, $target); >> + } >> + >> + $check_storage_access_migrate->($rpcenv, $authuser, $storecfg, $st= oragemap->{default}, $target) >> + if $storagemap->{default}; >> + >> + $param->{storagemap} =3D $storagemap; >> + } >> + >> if (PVE::HA::Config::vm_is_ha_managed($vmid) && $rpcenv->{type} ne 'ha= ') { >> =20 >> my $hacmd =3D 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 >=20 > ----8<---- >=20 >> @@ -194,7 +210,8 @@ sub phase1 { >> next if @{$dl->{$storeid}} =3D=3D 0; >> =20 >> # check if storage is available on target node >> - PVE::Storage::storage_check_enabled($self->{storecfg}, $storeid, $self= ->{node}); >> + my $targetsid =3D PVE::JSONSchema::map_id($self->{opts}->{storagemap},= $storeid); >> + PVE::Storage::storage_check_enabled($self->{storecfg}, $targetsid, $se= lf->{node}); >> =20 >> die "content type 'rootdir' is not available on storage '$storeid'\n" >> if !$scfg->{content}->{rootdir}; >=20 > Should use target's scfg. ack - had it in prepare (for the config-referenced volumes), but missed=20 the second one here. >=20 >> @@ -275,25 +292,38 @@ sub phase1 { >> next if $rep_volumes->{$volid}; >> my ($sid, $volname) =3D PVE::Storage::parse_volume_id($volid); >> push @{$self->{volumes}}, $volid; >> - my $bwlimit =3D PVE::Storage::get_bandwidth_limit('migration', [$sid],= $opts->{bwlimit}); >> + >> # JSONSchema and get_bandwidth_limit use kbps - storage_migrate bps >> + my $bwlimit =3D $volhash->{$volid}->{bwlimit}; >> $bwlimit =3D $bwlimit * 1024 if defined($bwlimit); >> =20 >> - my $storage_migrate_opts =3D { >> - 'ratelimit_bps' =3D> $bwlimit, >> - 'insecure' =3D> $opts->{migration_type} eq 'insecure', >> - 'with_snapshots' =3D> $volhash->{$volid}->{snapshots}, >> + my $targetsid =3D $volhash->{$volid}->{targetsid}; >> + >> + my $new_volid =3D eval { >> + my $storage_migrate_opts =3D { >> + 'ratelimit_bps' =3D> $bwlimit, >> + 'insecure' =3D> $opts->{migration_type} eq 'insecure', >> + 'with_snapshots' =3D> $volhash->{$volid}->{snapshots}, >=20 > 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 targe= t. true! >=20 >> + }; >> + >> + my $logfunc =3D sub { $self->log('info', $_[0]); }; >> + return PVE::Storage::storage_migrate( >> + $self->{storecfg}, >> + $volid, >> + $self->{ssh_info}, >> + $targetsid, >> + $storage_migrate_opts, >> + $logfunc, >> + ); >> }; >> =20 >> - my $logfunc =3D sub { $self->log('info', $_[0]); }; >> - eval { >> - PVE::Storage::storage_migrate($self->{storecfg}, $volid, $self->{s= sh_info}, >> - $sid, $storage_migrate_opts, $logfunc); >> - }; >> if (my $err =3D $@) { >> - die "storage migration for '$volid' to storage '$sid' failed - $er= r\n"; >> + die "storage migration for '$volid' to storage '$targetsid' failed= - $err\n"; >> } >> =20 >> + $self->{volume_map}->{$volid} =3D $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 =3D $@) { >> $self->log('warn', $err); >> @@ -316,6 +346,8 @@ sub phase1 { >> =20 >> # 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} =3D 1; >> $self->switch_replication_job_target() if $rep_volumes; >=20