all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: "Elias Huhsovitz" <e.huhsovitz@proxmox.com>
To: "Fiona Ebner" <f.ebner@proxmox.com>, <pve-devel@lists.proxmox.com>
Subject: Re: [PATCH qemu-server v2 7/7] clone disk: bump timeout for querying volume sizes to 1 minute
Date: Fri, 30 Oct 2026 12:14:12 +0100	[thread overview]
Message-ID: <DMI4NN80EM8W.3LJFUI2RG5TP0@proxmox.com> (raw)
In-Reply-To: <20261029120811.107814-8-f.ebner@proxmox.com>

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





      reply	other threads:[~2026-10-30 11:14 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-29 12:06 [PATCH-SERIES qemu-server v2 0/7] fix #8209: clone disk: do not ignore failure to query size of target Fiona Ebner
2026-10-29 12:06 ` [PATCH qemu-server v2 1/7] block job: monitor: document that job-complete is not used if source differs from target Fiona Ebner
2026-10-30 10:37   ` Elias Huhsovitz
2026-10-29 12:06 ` [PATCH qemu-server v2 2/7] drive: get tpm state size: also warn if there was no size returned Fiona Ebner
2026-10-30 10:39   ` Elias Huhsovitz
2026-10-29 12:06 ` [PATCH qemu-server v2 3/7] api: clone disk: downgrade failure to register unused volume to warning Fiona Ebner
2026-10-30 10:56   ` Elias Huhsovitz
2026-10-29 12:06 ` [PATCH qemu-server v2 4/7] fix #8209: clone disk: do not ignore failure to query size of target Fiona Ebner
2026-10-30 10:36   ` Elias Huhsovitz
2026-10-29 12:06 ` [PATCH qemu-server v2 5/7] clone disk: abort early when failing to query size of source disk Fiona Ebner
2026-10-29 12:06 ` [PATCH qemu-server v2 6/7] drive: get tpm state size: add timeout parameter Fiona Ebner
2026-10-30 11:02   ` Elias Huhsovitz
2026-10-29 12:06 ` [PATCH qemu-server v2 7/7] clone disk: bump timeout for querying volume sizes to 1 minute Fiona Ebner
2026-10-30 11:14   ` Elias Huhsovitz [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DMI4NN80EM8W.3LJFUI2RG5TP0@proxmox.com \
    --to=e.huhsovitz@proxmox.com \
    --cc=f.ebner@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal