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 EAB76A2C1 for ; Mon, 4 Apr 2022 17:27:04 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id E05B8FC0B for ; Mon, 4 Apr 2022 17:27:04 +0200 (CEST) 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 D8F6FFBC7 for ; Mon, 4 Apr 2022 17:27:03 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id A94B146251 for ; Mon, 4 Apr 2022 17:27:03 +0200 (CEST) Date: Mon, 04 Apr 2022 17:26:44 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox VE development discussion References: <20220401152424.3811621-1-a.lauterer@proxmox.com> <20220401152424.3811621-2-a.lauterer@proxmox.com> In-Reply-To: <20220401152424.3811621-2-a.lauterer@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.15.0 (https://github.com/astroidmail/astroid) Message-Id: <1649084728.7liqfd2nmz.astroid@nora.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL 0.177 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] [RFC qemu-server] clone disk: fix #3970 catch same source and destination 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: Mon, 04 Apr 2022 15:27:05 -0000 On April 1, 2022 5:24 pm, Aaron Lauterer wrote: > In rare situations, it could happen that the source and target path is > the same. For example, if the disk image is to be copied from one RBD > storage to another one on different Ceph clusters but the pools have the > same name. >=20 > In this situation, the clone operation will clone it to the same image an= d > one will end up with an empty destination volume. >=20 > This patch does not solve the underlying issue, but is a first step to > avoid potential data loss, for example when the 'delete source' option > is enabled as well. >=20 > We also need to delete the newly created image right away because the > regular cleanup gets confused and tries to remove the source image. This > will fail and we have an orphaned image which cannot be removed easily > because the same underlying root cause (same path) will falsely trigger > the "Drive::is_volume_in_use" check. isn't this technically - just like for the container case - a problem in=20 general, not just for cloning a disk? I haven't tested this in practice,=20 but since you already have the reproducing setup ;) e.g., given the following: - storage A, krbd, cluster A, pool foo - storage B, krbd, cluster B, pool foo - VM 123, with scsi0: A:vm-123-disk-0 and no volumes on B - qm set 123 -scsi1: B:1 next free slot on B is 'vm-123-disk-0', which will be allocated. mapping=20 will skip the map part, since the RBD path already exists (provided=20 scsi0's volume is already activated). the returned path will point to=20 the mapped blockdev corresponding to A:vm-123-disk-0, not B:..=20 guest writes to scsi1, likely corrupting whatever is on scsi0, since=20 most things that tend to get put on guest disks are not=20 multi-writer-safe (or something along the way notices it?) if the above is the case, it might actually be prudent to just put the=20 check from your other patch into RBDPlugin.pm 's alloc method (and=20 clone and rename?) since we'd want to block any allocations on affected=20 systems? >=20 > Signed-off-by: Aaron Lauterer > --- > We could think about leaving this as a permanent safety check as there > should not be a situation where both paths are the same AFAICT. >=20 > PVE/QemuServer.pm | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) >=20 > diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm > index 6c80c0c..a1aa4f2 100644 > --- a/PVE/QemuServer.pm > +++ b/PVE/QemuServer.pm > @@ -7633,6 +7633,17 @@ sub clone_disk { > =20 > PVE::Storage::activate_volumes($storecfg, [$newvolid]); > =20 > + my $src_path =3D PVE::Storage::path($storecfg, $drive->{file}); > + my $dst_path =3D PVE::Storage::path($storecfg, $newvolid); > + > + if ($src_path eq $dst_path) { > + # Delete the new volume right away. Doing it later will try to remo= ve the old volume and > + # fail. Currenlty the only known case is RBD. See bugs 3969 and 397= 0 for more details. > + PVE::Storage::vdisk_free($storecfg, $newvolid); > + pop @$newvollist; > + die "Source and destination are the same!\n"; > + } > + > if (drive_is_cloudinit($drive)) { > # when cloning multiple disks (e.g. during clone_vm) it might be th= e last disk > # if this is the case, we have to complete any block-jobs still the= re from > @@ -7650,8 +7661,6 @@ sub clone_disk { > # the relevant data on the efidisk may be smaller than the source > # e.g. on RBD/ZFS, so we use dd to copy only the amount > # that is given by the OVMF_VARS.fd > - my $src_path =3D PVE::Storage::path($storecfg, $drive->{file}); > - my $dst_path =3D PVE::Storage::path($storecfg, $newvolid); > =20 > # better for Ceph if block size is not too small, see bug #3324 > my $bs =3D 1024*1024; > --=20 > 2.30.2 >=20 >=20 >=20 > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel >=20 >=20 >=20