From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 1C1B41FF0AF for ; Thu, 22 Oct 2026 11:39:30 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 8778821645; Thu, 22 Oct 2026 11:39:24 +0200 (CEST) From: Fiona Ebner To: pve-devel@lists.proxmox.com Subject: [PATCH qemu-server 2/4] fix #8209: clone disk: do not ignore failure to query size of target Date: Thu, 22 Oct 2026 11:38:16 +0200 Message-ID: <20261022093901.52732-3-f.ebner@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20261022093901.52732-1-f.ebner@proxmox.com> References: <20261022093901.52732-1-f.ebner@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1792661959831 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.343 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: 4HUVY335DU32LQPPBGQIB3T3KW7IFEDL X-Message-ID-Hash: 4HUVY335DU32LQPPBGQIB3T3KW7IFEDL X-MailFrom: f.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Since commit 3bae384f ("clone disk: avoid errors after disk was moved by QEMU"), failure to query the target volume's size after clone is ignored. It's sensible not to fail hard at this stage, because in case of live mirror for the same VM with 'complete' completion mode, the drive was already switched. Also, the error might be a harmless timeout. Still, as reported in bug #8209, there could be an actual issue, so log a warning and communicate a failure to the caller, so it can avoid destructive action. Some storage plugin implementations of the volume_size_info() method do not fail with a hard error. In particular, the native file-based plugins use file_size_info() which returns undef upon failure. Thus, explicitly check the size. Signed-off-by: Fiona Ebner --- src/PVE/API2/Qemu.pm | 21 ++++++++++++++------- src/PVE/QemuServer.pm | 14 +++++++++++++- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm index f4dbd2a1..8e7ece52 100644 --- a/src/PVE/API2/Qemu.pm +++ b/src/PVE/API2/Qemu.pm @@ -432,7 +432,7 @@ my $import_from_volid = sub { $cloned = $clonefn->(); } - return $cloned->@{qw(file size)}; + return $cloned->{drive}->@{qw(file size)}; }; my sub prohibit_tpm_version_change { @@ -4703,7 +4703,7 @@ __PACKAGE__->register_method({ $dest_info->{efisize} = PVE::QemuServer::get_efivars_size($oldconf) if $opt eq 'efidisk0'; - my $newdrive = PVE::QemuServer::clone_disk( + my $clone_result = PVE::QemuServer::clone_disk( $storecfg, $source_info, $dest_info, @@ -4715,7 +4715,7 @@ __PACKAGE__->register_method({ $clonelimit, ); - $newconf->{$opt} = PVE::QemuServer::print_drive($newdrive); + $newconf->{$opt} = PVE::QemuServer::print_drive($clone_result->{drive}); PVE::QemuConfig->write_config($newid, $newconf); $i++; @@ -4900,6 +4900,7 @@ __PACKAGE__->register_method({ my $target_disk = extract_param($param, 'target-disk') // $disk; my $storeid = extract_param($param, 'storage'); my $format = extract_param($param, 'format'); + my $do_delete = extract_param($param, 'delete'); my $storecfg = PVE::Storage::config(); @@ -4932,7 +4933,7 @@ __PACKAGE__->register_method({ $storecfg, $conf, $disk, $old_volid, ); die "you can't move a disk with snapshots and delete the source\n" - if $snapshotted && $param->{delete}; + if $snapshotted && $do_delete; return ($conf, $drive, $oldstoreid, $snapshotted); }; @@ -4986,7 +4987,7 @@ __PACKAGE__->register_method({ $dest_info->{efisize} = PVE::QemuServer::get_efivars_size($conf) if $disk eq 'efidisk0'; - my $newdrive = PVE::QemuServer::clone_disk( + my $clone_result = PVE::QemuServer::clone_disk( $storecfg, $source_info, $dest_info, @@ -4997,9 +4998,15 @@ __PACKAGE__->register_method({ $conf->{agent}, $movelimit, ); + my $newdrive = $clone_result->{drive}; $conf->{$disk} = PVE::QemuServer::print_drive($newdrive); - if (!$param->{delete}) { + if ($do_delete && $clone_result->{'after-copy-error'}) { + warn "skipping delete of source because of error after copy\n"; + $do_delete = 0; + } + + if (!$do_delete) { my $unused_key = PVE::QemuConfig->add_unused_volume($conf, $old_volid); print("adding source volume '$old_volid' as '$unused_key' to config\n"); } @@ -5030,7 +5037,7 @@ __PACKAGE__->register_method({ die "storage migration failed: $err"; } - if ($param->{delete}) { + if ($do_delete) { print("removing source volume '$old_volid' after successful copy\n"); eval { PVE::Storage::deactivate_volumes($storecfg, [$old_volid]); diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm index 1efeeb66..8f062466 100644 --- a/src/PVE/QemuServer.pm +++ b/src/PVE/QemuServer.pm @@ -7929,14 +7929,26 @@ sub clone_disk { } } + my $result = {}; my $size = eval { PVE::Storage::volume_size_info($storecfg, $newvolid, 10) }; + my $err = $@; + $err = "no size in result" if !$err && !$size; + if ($err) { + # The failure might be a harmless timeout. In case of live mirror for the same VM with + # 'complete' completion mode, the drive was already switched. Thus, avoid a hard error at + # this stage. Still, there could be an actual issue, so the caller can avoid destructive + # action based on the presence of 'after-copy-error'. + log_warn("unable to query size for '$newvolid' after copy - $err"); + $result->{'after-copy-error'} = 1; + } my $disk = dclone($drive); delete $disk->{format}; $disk->{file} = $newvolid; $disk->{size} = $size if defined($size) && !$unused; - return $disk; + $result->{drive} = $disk; + return $result; } sub qemu_use_old_bios_files { -- 2.47.3