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

[Fiona Ebner <f.ebner@proxmox.com>]

Am 23.10.26 um 11:31 AM schrieb Elias Huhsovitz:
> On Thu Oct 22, 2026 at 11:38 AM CEST, 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
>> 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";
> 
> nit: IMO the message should say: 'after clone...', since we are calling 
> PVE::Storage::vdisk_clone.

The message is user-facing, so it does not matter what the function in
Perl is called. But we do log "create full|linked clone of drive ..." in
the beginning of the function, so I'll change it to be consistent with
that :)

> 
> super nit: i think it should be three dots (...), instead of 2 (..) ;)

We do have quite some instances with .. but I'll change it.

> 
>> +    my $size = eval { PVE::Storage::volume_size_info($storecfg, $newvolid, 60) };
> 
> nit: The timeout is inconsistent with the volume_size_info() call on
> line 7897. There it is still 10 seconds. I am not sure if we can
> gurantee that querying the source volume is always faster than querying
> the target volume.

That's fair, especially since there can be multiple disks involved in a
full guest clone. I'll bump it there too.

> general: Tbh I am not sure that a generic timeout is the optimal
> long-term solution here. I feel like the timeout should be storage
> specific. E.g., an incrased timeout of 60 seconds for a NFS, but keep
> the 10 seconds for a local ZFS.

I don't think we should try to predict how fast a storage "should" be
based on the plugin type. That depends much more on the
hardware/workloads. Also, clone is already a long-running operation and
failure can be a lot of wasted work, so I don't think trying to
"optimize" the tolerated timeout would be useful. If a storage is fast,
it will just not run into the timeout; if it is slow, the larger timeout
is beneficial; if it is usually fast, but not right now, it is still
beneficial to have the larger timeout.

> 
> But for now, the incrased timeout is fine.
> 
>>      my $err = $@;
>>      $err = "no size in result" if !$err && !$size;
>>      if ($err) {
>