[PATCH-SERIES qemu-server 0/4] fix #8209: clone disk: do not ignore failure to query size of target

[PATCH-SERIES qemu-server 0/4] fix #8209: clone disk: do not ignore failure to query size of target

[Fiona Ebner]

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.

qemu-server:

Fiona Ebner (4):
  block job: monitor: document that job-complete is not used if source
    differs from target
  fix #8209: clone disk: do not ignore failure to query size of target
  clone disk: abort early when failing to query size of source disk
  clone disk: bump timeout for querying target volume size to 1 minute

 src/PVE/API2/Qemu.pm           | 21 ++++++++++++++-------
 src/PVE/QemuServer.pm          | 18 ++++++++++++++++--
 src/PVE/QemuServer/BlockJob.pm |  1 +
 3 files changed, 31 insertions(+), 9 deletions(-)


Summary over all repositories:
  3 files changed, 31 insertions(+), 9 deletions(-)

-- 
Generated by git-murpp 0.5.0

[PATCH qemu-server 1/4] block job: monitor: document that job-complete is not used if source differs from target

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuServer/BlockJob.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/PVE/QemuServer/BlockJob.pm b/src/PVE/QemuServer/BlockJob.pm
index 493dda33..5757c24f 100644
--- a/src/PVE/QemuServer/BlockJob.pm
+++ b/src/PVE/QemuServer/BlockJob.pm
@@ -82,6 +82,7 @@ sub qemu_blockjobs_cancel {
 
 # $completion can be either
 # 'complete': wait until all jobs are ready, job-complete them (default)
+#     If the source peer is not the same as the destination VM, uses block-job-cancel instead.
 # 'cancel': wait until all jobs are ready, block-job-cancel them
 # 'skip': wait until all jobs are ready, return with block jobs in ready state
 # 'auto': wait until all jobs disappear, only use for jobs which complete automatically
-- 
2.47.3

[PATCH qemu-server 2/4] fix #8209: clone disk: do not ignore failure to query size of target

[Fiona Ebner]

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 <f.ebner@proxmox.com>
---
 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

[PATCH qemu-server 3/4] clone disk: abort early when failing to query size of source disk

[Fiona Ebner]

Not all plugin implementations of the volume_size_info() method fail
if there is an error. In particular, the native file-based plugins
use file_size_info() which returns undef upon failure.

While clone is likely to fail later (e.g. vdisk_alloc() is called with
an undef size), avoid any surprises and fail early with a clean error.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuServer.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 8f062466..4b7351b3 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -7895,6 +7895,7 @@ sub clone_disk {
             );
 
             $size = PVE::Storage::volume_size_info($storecfg, $drive->{file}, 10);
+            die "unable to query size of volume '$drive->{file}'\n" if !$size;
         }
         $source->{size} = $size;
         $newvolid = PVE::Storage::vdisk_alloc(
-- 
2.47.3

[PATCH qemu-server 4/4] clone disk: bump timeout for querying target volume size to 1 minute

[Fiona Ebner]

Since commit "fix #8209: clone disk: do not ignore failure to query
size of target", failure to query the size of the target volume after
clone is no longer quietly ignored. Bump the timeout to reduce the
likelyhood of harmless timeouts causing a warning. Print a message to
communicate that the query is happening to the user.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuServer.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 4b7351b3..0f8aef6f 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -7931,7 +7931,8 @@ sub clone_disk {
     }
 
     my $result = {};
-    my $size = eval { PVE::Storage::volume_size_info($storecfg, $newvolid, 10) };
+    print "querying size of target volume '$newvolid' after copy..\n";
+    my $size = eval { PVE::Storage::volume_size_info($storecfg, $newvolid, 60) };
     my $err = $@;
     $err = "no size in result" if !$err && !$size;
     if ($err) {
-- 
2.47.3

Re: [PATCH qemu-server 2/4] fix #8209: clone disk: do not ignore failure to query size of target

[Elias Huhsovitz]

Comments inline

(This is not a full review yet)

On Thu Oct 22, 2026 at 11:38 AM CEST, Fiona Ebner wrote:
> 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 <f.ebner@proxmox.com>
> ---
>  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;

Since we now set $clone_result->{'after-copy-error'}, if e.g., the
target NFS share returns an error, we set $do_delete = 0 to prevent the
source disk from being deleted.

> +                }
> +
> +                if (!$do_delete) {
>                      my $unused_key = PVE::QemuConfig->add_unused_volume($conf, $old_volid);

So now in the case that $clone_result->{'after-copy-error'} is set, we
always call add_unused_volume(). But add_unused_volume() can fail with

"Too many unused volumes - please delete them first.\n"

according to AbstractConfig::add_unused_volume()

Couldn't this kill the disk during a live migration, if we are unable to
add the unused volume?

In Drive.pm the maximum number for unused disks is set to:

our $MAX_UNUSED_DISKS = 256;

IMO being unable to add a new disk in a production system, might be a
real possibility.

>                      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 {