all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: "Jakob Klocker" <j.klocker@proxmox.com>
To: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>,
	pve-devel@lists.proxmox.com
Subject: Re: [PATCH storage] fix #7598: qemu-img resize: tolerate timeout if resize succeeded
Date: Mon, 27 Jul 2026 14:50:20 +0200	[thread overview]
Message-ID: <DK9D7HRNPY85.MVVSQA6P68GN@proxmox.com> (raw)
In-Reply-To: <1784886368.5u9kt3tdeb.astroid@yuna.none>

Thanks for the feedback. Replies inline

On Fri Jul 24, 2026 at 11:57 AM CEST, Fabian Grünbichler wrote:
> On June 3, 2026 10:25 am, Jakob Klocker wrote:
>> On slow storages there is a chance the 10 second timeout is triggered
>> when resizing a volume. If the timeout fires while the resize is in a
>> certain state near the end, the operation can still complete
>> successfully even though a timeout error is thrown. In that case the
>> config is never updated and keeps the old, wrong size.
>> 
>> Because the config is out of sync, the volume is then displayed with
>> the wrong size in the web interface.
>> 
>>[SNIP]
>>  
>>      $timeout = 10 if !$timeout;
>
> I think making this worker-task aware and bumping the timeout in that
> case would make more sense - storage performance varies wildy, and like
> I noted in the bug, this only seems to be called from a worker context
> where the additional time hurts way less than risking running into the
> timeout just because something is slow.
>
> e.g., in the RBD plugin we have a default connect timeout of 60s in
> workers, in the ZFS plugin we bump ZFS requests to 300s if a smaller
> timeout is set, and default to a timeout of one hour if none is set.
>

I'll mirror the ZFS pluign in a v2 and default the timeout to an hour in
worker context.

>> -    run_command($cmd, timeout => $timeout);
>> +    eval { run_command($cmd, timeout => $timeout); };
>> +    if (my $err = $@) {
>> +
>> +        die $err if $err !~ /got timeout/;
>> +
>> +        my $info = JSON::decode_json(qemu_img_info($path, $format));
>> +        die $err if !$info;
>> +
>> +        my $actual_size = $info->{'virtual-size'};
>> +        die $err if !defined($actual_size) || $actual_size < $size;
>> +    }
>
> IMHO this doesn't fix the actual bug mentioned.. I guess what you see
> here if running into this behaviour is that the resize was done, but
> syncing then takes long and hits the timeout?
>
> because what `qemu-img resize` does for raw images is basically just
>
> ftruncate(..)
> fdatasync(..)
>
> and just because you read back the updated size in the timeout case
> (from local, cached metadata), does not necessarily mean it got
> persisted to the storage (which might be on a different system) - that's
> what the sync is for after all..
>

What concerned me most in the bug report was the state after a failed 
resize, where a subsequent relative resize would grow the disk further 
than intended - the reporter's 2G disk ending up at 32G after two +16G
requests, even though the first one appeared to fail [0].

I reproduced this and what happened is:

The first resize runs into the timeout. The resize did take effect on
the storage, but because the command was reported as failed, the size
property in the VM config was never updated - it still reads the old
value.

The second resize reads the actual on-disk size (not the stale config
value), so it computes the correct target and succeeds. In doing so it
also refreshes the config's size property to the storage's current size
- which by now reflects both resizes, hence the jump to 32G.


Since I missed that the read-back size isn't necessarily the persisted
size, there's no real way to deterministically tell whether the resize was
actually persisted. Therefore I'll bump the timeout as you suggested. 
That doesn't make the bug strictly impossible - if a resize still hits
the (much larger) worker timeout, the same stale-config situation could
occur. But with the timeout raised that should barely be the case.

Thanks for pointing out the sync issue!


[0] https://bugzilla.proxmox.com/show_bug.cgi?id=7598#c3

>>  }
>>  
>>  1;
>> -- 
>> 2.47.3
>> 
>> 
>> 
>> 
>> 





      reply	other threads:[~2026-07-27 12:50 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-03  8:25 [PATCH storage] fix #7598: qemu-img resize: tolerate timeout if resize succeeded Jakob Klocker
2026-07-24  9:57 ` Fabian Grünbichler
2026-07-27 12:50   ` Jakob Klocker [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=DK9D7HRNPY85.MVVSQA6P68GN@proxmox.com \
    --to=j.klocker@proxmox.com \
    --cc=f.gruenbichler@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