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

["Elias Huhsovitz" <e.huhsovitz@proxmox.com>]

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 {