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

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

[Fiona Ebner]

Changes in v2:
* Rebase on current master.
* Bump timeout also for querying size of source volume.
* Add patch to fix issue when adding unused volume fails.
* Improve log output.

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 (7):
  block job: monitor: document that job-complete is not used if source
    differs from target
  drive: get tpm state size: also warn if there was no size returned
  api: clone disk: downgrade failure to register unused volume to
    warning
  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
  drive: get tpm state size: add timeout parameter
  clone disk: bump timeout for querying volume sizes to 1 minute

 src/PVE/API2/Qemu.pm           | 31 ++++++++++++++++++++++---------
 src/PVE/QemuServer.pm          | 24 ++++++++++++++++++++----
 src/PVE/QemuServer/BlockJob.pm |  1 +
 src/PVE/QemuServer/Drive.pm    | 13 +++++++++----
 4 files changed, 52 insertions(+), 17 deletions(-)


Summary over all repositories:
  4 files changed, 52 insertions(+), 17 deletions(-)

-- 
Generated by git-murpp 0.5.0

[PATCH qemu-server v2 1/7] 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>
---

No changes in v2.

 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

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

[Elias Huhsovitz]

Reviewed-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>

On Thu Oct 29, 2026 at 1:06 PM CET, Fiona Ebner wrote:
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>
> No changes in v2.
>
>  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

[PATCH qemu-server v2 2/7] drive: get tpm state size: also warn if there was no size returned

[Fiona Ebner]

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

New in v2.

 src/PVE/QemuServer/Drive.pm | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/PVE/QemuServer/Drive.pm b/src/PVE/QemuServer/Drive.pm
index 0ca3d636..7b288e78 100644
--- a/src/PVE/QemuServer/Drive.pm
+++ b/src/PVE/QemuServer/Drive.pm
@@ -1334,7 +1334,9 @@ sub get_tpmstate_size {
         if !defined($storecfg);
 
     my $size = eval { PVE::Storage::volume_size_info($storecfg, $volid, 10) };
-    warn "failed to get size of the tpm drive $volid: $@" if $@;
+    my $err = $@;
+    $err = "no size in result" if !$err && !$size;
+    warn "failed to get size of the tpm drive $volid: $err" if $err;
 
     if (!$size || $size < TPMSTATE_DISK_SIZE) {
         $size = TPMSTATE_DISK_SIZE;
-- 
2.47.3

Re: [PATCH qemu-server v2 2/7] drive: get tpm state size: also warn if there was no size returned

[Elias Huhsovitz]

More specific error messages make debugging easier, so consider this:

Reviewed-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>

On Thu Oct 29, 2026 at 1:06 PM CET, Fiona Ebner wrote:
> 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>
> ---
>
> New in v2.
>
>  src/PVE/QemuServer/Drive.pm | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/PVE/QemuServer/Drive.pm b/src/PVE/QemuServer/Drive.pm
> index 0ca3d636..7b288e78 100644
> --- a/src/PVE/QemuServer/Drive.pm
> +++ b/src/PVE/QemuServer/Drive.pm
> @@ -1334,7 +1334,9 @@ sub get_tpmstate_size {
>          if !defined($storecfg);
>  
>      my $size = eval { PVE::Storage::volume_size_info($storecfg, $volid, 10) };
> -    warn "failed to get size of the tpm drive $volid: $@" if $@;
> +    my $err = $@;
> +    $err = "no size in result" if !$err && !$size;
> +    warn "failed to get size of the tpm drive $volid: $err" if $err;
>  
>      if (!$size || $size < TPMSTATE_DISK_SIZE) {
>          $size = TPMSTATE_DISK_SIZE;

[PATCH qemu-server v2 3/7] api: clone disk: downgrade failure to register unused volume to warning

[Fiona Ebner]

When reaching the call to register the source volume as unused after
clone, a running VM will already be switched over to the new disk. If
an error occurs there (because all unused slots are already in use),
the new disk would be removed in the error handling. Even if it is an
offline clone, the failure to register the source volume as unused
should not lead to aborting/reversing the whole operation. Downgrade
the error to a warning.

Reported-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

New in v2.

 src/PVE/API2/Qemu.pm | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
index ebbfcb1b..9540db86 100644
--- a/src/PVE/API2/Qemu.pm
+++ b/src/PVE/API2/Qemu.pm
@@ -5012,8 +5012,12 @@ __PACKAGE__->register_method({
                 $conf->{$disk} = PVE::QemuServer::print_drive($newdrive);
 
                 if (!$param->{delete}) {
-                    my $unused_key = PVE::QemuConfig->add_unused_volume($conf, $old_volid);
-                    print("adding source volume '$old_volid' as '$unused_key' to config\n");
+                    eval {
+                        my $unused_key = PVE::QemuConfig->add_unused_volume($conf, $old_volid);
+                        print("adding source volume '$old_volid' as '$unused_key' to config\n");
+                    };
+                    log_warn("cannot add source volume '$old_volid' as unused to config - $@")
+                        if $@;
                 }
 
                 # convert moved disk to base if part of template
-- 
2.47.3

Re: [PATCH qemu-server v2 3/7] api: clone disk: downgrade failure to register unused volume to warning

[Elias Huhsovitz]

I like the approach. I was also considering to simply check the number of
slots used before the actual cloning of the disk, but then we might just
encounter a new edge case down the line... So IMO this solution is
better :D

Reviewed-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>

On Thu Oct 29, 2026 at 1:06 PM CET, Fiona Ebner wrote:
> When reaching the call to register the source volume as unused after
> clone, a running VM will already be switched over to the new disk. If
> an error occurs there (because all unused slots are already in use),
> the new disk would be removed in the error handling. Even if it is an
> offline clone, the failure to register the source volume as unused
> should not lead to aborting/reversing the whole operation. Downgrade
> the error to a warning.
>
> Reported-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>
> New in v2.
>
>  src/PVE/API2/Qemu.pm | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/src/PVE/API2/Qemu.pm b/src/PVE/API2/Qemu.pm
> index ebbfcb1b..9540db86 100644
> --- a/src/PVE/API2/Qemu.pm
> +++ b/src/PVE/API2/Qemu.pm
> @@ -5012,8 +5012,12 @@ __PACKAGE__->register_method({
>                  $conf->{$disk} = PVE::QemuServer::print_drive($newdrive);
>  
>                  if (!$param->{delete}) {
> -                    my $unused_key = PVE::QemuConfig->add_unused_volume($conf, $old_volid);
> -                    print("adding source volume '$old_volid' as '$unused_key' to config\n");
> +                    eval {
> +                        my $unused_key = PVE::QemuConfig->add_unused_volume($conf, $old_volid);
> +                        print("adding source volume '$old_volid' as '$unused_key' to config\n");
> +                    };
> +                    log_warn("cannot add source volume '$old_volid' as unused to config - $@")
> +                        if $@;
>                  }
>  
>                  # convert moved disk to base if part of template

[PATCH qemu-server v2 4/7] 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>
---

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

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

[Elias Huhsovitz]

Gave this a quick test by:

1. making PVE::Storage::volume_size_info reach timeout (via artificial
   sleep)
2. moving disk: lvmthin -> local-zfs with delete source enabled

Result:

1. Original soure volume was was added to config as 'unused0'.
2. VM booted as normal with new disk on local-zfs. This might fail in a
real env, so I:
3. re-atteched the original lvm-thin source disk (unused0)
4. enabled it in the boot order menu.
5. Original source disk also booted VM without an issue.

Changes seem good to me!

Reviewed-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
Tested-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>

On Thu Oct 29, 2026 at 1:06 PM CET, 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>
> ---
>
> 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 {

[PATCH qemu-server v2 5/7] 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.

Reviewed-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>
Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

No changes in v2.

 src/PVE/QemuServer.pm | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index d92b2e95..636a5602 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -7952,6 +7952,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 v2 6/7] drive: get tpm state size: add timeout parameter

[Fiona Ebner]

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

New in v2.

 src/PVE/QemuServer/Drive.pm | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/PVE/QemuServer/Drive.pm b/src/PVE/QemuServer/Drive.pm
index 7b288e78..951b7bb6 100644
--- a/src/PVE/QemuServer/Drive.pm
+++ b/src/PVE/QemuServer/Drive.pm
@@ -1323,9 +1323,12 @@ sub update_tpmstate_size {
 
 # Takes in a volume id and the storage configuration. Will then try to find
 # the actual storage size of the TPM drive. The final value has a minimum
-# determined by the TPMSTATE_DISK_SIZE static, which is 4MiB.
+# determined by the TPMSTATE_DISK_SIZE static, which is 4MiB. The default
+# timeout is 10 seconds and can be changed via the $timeout parameter.
 sub get_tpmstate_size {
-    my ($volid, $storecfg) = @_;
+    my ($volid, $storecfg, $timeout) = @_;
+
+    $timeout = 10 if !defined($timeout);
 
     die "a volume id has to be supplied to determine tpm state size"
         if !defined($volid);
@@ -1333,7 +1336,7 @@ sub get_tpmstate_size {
     die "the storage config has to be supplied to determine tpm state size"
         if !defined($storecfg);
 
-    my $size = eval { PVE::Storage::volume_size_info($storecfg, $volid, 10) };
+    my $size = eval { PVE::Storage::volume_size_info($storecfg, $volid, $timeout) };
     my $err = $@;
     $err = "no size in result" if !$err && !$size;
     warn "failed to get size of the tpm drive $volid: $err" if $err;
-- 
2.47.3

Re: [PATCH qemu-server v2 6/7] drive: get tpm state size: add timeout parameter

[Elias Huhsovitz]

LGTM!

Reviewed-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>

On Thu Oct 29, 2026 at 1:06 PM CET, Fiona Ebner wrote:
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>
> New in v2.
>
>  src/PVE/QemuServer/Drive.pm | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/src/PVE/QemuServer/Drive.pm b/src/PVE/QemuServer/Drive.pm
> index 7b288e78..951b7bb6 100644
> --- a/src/PVE/QemuServer/Drive.pm
> +++ b/src/PVE/QemuServer/Drive.pm
> @@ -1323,9 +1323,12 @@ sub update_tpmstate_size {
>  
>  # Takes in a volume id and the storage configuration. Will then try to find
>  # the actual storage size of the TPM drive. The final value has a minimum
> -# determined by the TPMSTATE_DISK_SIZE static, which is 4MiB.
> +# determined by the TPMSTATE_DISK_SIZE static, which is 4MiB. The default
> +# timeout is 10 seconds and can be changed via the $timeout parameter.
>  sub get_tpmstate_size {
> -    my ($volid, $storecfg) = @_;
> +    my ($volid, $storecfg, $timeout) = @_;
> +
> +    $timeout = 10 if !defined($timeout);
>  
>      die "a volume id has to be supplied to determine tpm state size"
>          if !defined($volid);
> @@ -1333,7 +1336,7 @@ sub get_tpmstate_size {
>      die "the storage config has to be supplied to determine tpm state size"
>          if !defined($storecfg);
>  
> -    my $size = eval { PVE::Storage::volume_size_info($storecfg, $volid, 10) };
> +    my $size = eval { PVE::Storage::volume_size_info($storecfg, $volid, $timeout) };
>      my $err = $@;
>      $err = "no size in result" if !$err && !$size;
>      warn "failed to get size of the tpm drive $volid: $err" if $err;

[PATCH qemu-server v2 7/7] clone disk: bump timeout for querying volume sizes 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
likelihood of harmless timeouts causing a warning. Print a message to
communicate that the query is happening to the user.

For consistency, also bump the timeout when querying the source
volume. Clone might involve multiple disks and it could be a lot of
wasted work if the whole operation is aborted because of reaching the
timeout when querying the size.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Changes in v2:
* Use 'clone' in log message for consistency and triple dots.
* Also bump timeout for querying source volumes.

 src/PVE/QemuServer.pm | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 636a5602..e1a0ad2f 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -7945,13 +7945,15 @@ sub clone_disk {
         } elsif ($dst_drivename eq 'efidisk0') {
             $size = $efisize or die "internal error - need to specify EFI disk size\n";
         } elsif ($dst_drivename eq 'tpmstate0') {
-            $size = get_tpmstate_size($drive->{file}, $storecfg);
+            print "querying size of source volume '$drive->{file}'...\n";
+            $size = get_tpmstate_size($drive->{file}, $storecfg, 60);
         } else {
             clone_disk_check_io_uring(
                 $vmid, $drive, $storecfg, $src_storeid, $storeid, $use_drive_mirror,
             );
 
-            $size = PVE::Storage::volume_size_info($storecfg, $drive->{file}, 10);
+            print "querying size of source volume '$drive->{file}'...\n";
+            $size = PVE::Storage::volume_size_info($storecfg, $drive->{file}, 60);
             die "unable to query size of volume '$drive->{file}'\n" if !$size;
         }
         $source->{size} = $size;
@@ -7988,7 +7990,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 clone...\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 v2 7/7] clone disk: bump timeout for querying volume sizes to 1 minute

[Elias Huhsovitz]

I like the additional verbosity, but I feel like this should belong 
in logs with log level = DEBUG. But that would require a big upgrade to
our logging system. So this change LGTM for now!

side note: We seem to have a lot of hard-coded timeout values in this
repo. Perhaps we should consider parameterizing them somehow, e.g. to
make them configureable by advanced users.

Just my thoughts, doesn't effect this review, consider this:

Reviewed-by: Elias Huhsovitz <e.huhsovitz@proxmox.com>

On Thu Oct 29, 2026 at 1:06 PM CET, Fiona Ebner wrote:
> 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
> likelihood of harmless timeouts causing a warning. Print a message to
> communicate that the query is happening to the user.
>
> For consistency, also bump the timeout when querying the source
> volume. Clone might involve multiple disks and it could be a lot of
> wasted work if the whole operation is aborted because of reaching the
> timeout when querying the size.
>
> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
> ---
>
> Changes in v2:
> * Use 'clone' in log message for consistency and triple dots.
> * Also bump timeout for querying source volumes.
>
>  src/PVE/QemuServer.pm | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
> index 636a5602..e1a0ad2f 100644
> --- a/src/PVE/QemuServer.pm
> +++ b/src/PVE/QemuServer.pm
> @@ -7945,13 +7945,15 @@ sub clone_disk {
>          } elsif ($dst_drivename eq 'efidisk0') {
>              $size = $efisize or die "internal error - need to specify EFI disk size\n";
>          } elsif ($dst_drivename eq 'tpmstate0') {
> -            $size = get_tpmstate_size($drive->{file}, $storecfg);
> +            print "querying size of source volume '$drive->{file}'...\n";
> +            $size = get_tpmstate_size($drive->{file}, $storecfg, 60);
>          } else {
>              clone_disk_check_io_uring(
>                  $vmid, $drive, $storecfg, $src_storeid, $storeid, $use_drive_mirror,
>              );
>  
> -            $size = PVE::Storage::volume_size_info($storecfg, $drive->{file}, 10);
> +            print "querying size of source volume '$drive->{file}'...\n";
> +            $size = PVE::Storage::volume_size_info($storecfg, $drive->{file}, 60);
>              die "unable to query size of volume '$drive->{file}'\n" if !$size;
>          }
>          $source->{size} = $size;
> @@ -7988,7 +7990,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 clone...\n";
> +    my $size = eval { PVE::Storage::volume_size_info($storecfg, $newvolid, 60) };
>      my $err = $@;
>      $err = "no size in result" if !$err && !$size;
>      if ($err) {