From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 64FAA1FF0BC for ; Thu, 29 Oct 2026 13:09:10 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 1F0ED2168B; Thu, 29 Oct 2026 13:08:28 +0100 (CET) From: Fiona Ebner To: pve-devel@lists.proxmox.com Subject: [PATCH qemu-server v2 4/7] fix #8209: clone disk: do not ignore failure to query size of target Date: Thu, 29 Oct 2026 13:06:55 +0100 Message-ID: <20261029120811.107814-5-f.ebner@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20261029120811.107814-1-f.ebner@proxmox.com> References: <20261029120811.107814-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: 1793275700970 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.325 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: CGWP7DRSZXVTDRTT7BHYGSM525M25FEN X-Message-ID-Hash: CGWP7DRSZXVTDRTT7BHYGSM525M25FEN 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 --- Changes in v2: * Include source volume ID in warning message. * Use log_warn(). src/PVE/API2/Qemu.pm | 23 ++++++++++++++++------- src/PVE/QemuServer.pm | 14 +++++++++++++- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm index 9540db86..72471d3b 100644 --- a/src/PVE/API2/Qemu.pm +++ b/src/PVE/API2/Qemu.pm @@ -436,7 +436,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 { @@ -4712,7 +4712,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, @@ -4724,7 +4724,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++; @@ -4912,6 +4912,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(); @@ -4944,7 +4945,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); }; @@ -4998,7 +4999,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, @@ -5009,9 +5010,17 @@ __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'}) { + log_warn( + "skipping delete of source volume '$old_volid' because of error after copy" + ); + $do_delete = 0; + } + + if (!$do_delete) { eval { my $unused_key = PVE::QemuConfig->add_unused_volume($conf, $old_volid); print("adding source volume '$old_volid' as '$unused_key' to config\n"); @@ -5046,7 +5055,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 b4a1a38d..d92b2e95 100644 --- a/src/PVE/QemuServer.pm +++ b/src/PVE/QemuServer.pm @@ -7986,14 +7986,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