public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Fiona Ebner <f.ebner@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH qemu-server 1/1] block resize: avoid passing zero size to QMP command
Date: Fri, 28 Apr 2023 14:32:07 +0200	[thread overview]
Message-ID: <20230428123209.124097-2-f.ebner@proxmox.com> (raw)
In-Reply-To: <20230428123209.124097-1-f.ebner@proxmox.com>

Commit 7246e8f9 ("Set zero $size and continue if volume_resize()
returns false") mentions that this is needed for "some storages with
backing block devices to do online resize" and since this patch came
together [0] with pve-storage commit a4aee43 ("Fix RBD resize with
krbd option enabled."), it's safe to assume that RBD with krbd is
meant. But it should be the same situation for any external plugin
relying on the same behavior.

Other storages backed by block devices like LVM(-thin) and ZFS return
1 and the new size respectively, and the code is older than the above
mentioned commits. So really, the RBD plugin just should have returned
a positive value to be in-line with those and there should be no need
to pass 0 to the block_resize QMP command either.

Actually, it's a hack, because the block_resize QMP command does not
actually do special handling for the value 0. It's just that in the
case of a block device, QEMU won't try to resize it (and not fail for
shrinkage). But the size in the raw driver's BlockDriverState is
temporarily set to 0 (which is not nice), until the sector count is
refreshed, where raw_co_getlength is called, which queries the new
size and sets the size in the raw driver's BlockDriverState again as a
side effect. It's not known to cause any issues, but bdrv_getlength is
a coroutine wrapper starting from QEMU 8.0.0, and it's just better to
avoid setting a completely wrong value even temporarily. Just pass the
actually requested size like is done for LVM(thin) and ZFS.

[0]: https://lists.proxmox.com/pipermail/pve-devel/2017-January/025060.html

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 PVE/QemuServer.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index c1d0fd2d..86d88914 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -4766,7 +4766,7 @@ sub qemu_block_resize {
 
     my $running = check_running($vmid);
 
-    $size = 0 if !PVE::Storage::volume_resize($storecfg, $volid, $size, $running);
+    PVE::Storage::volume_resize($storecfg, $volid, $size, $running);
 
     return if !$running;
 
-- 
2.30.2





  reply	other threads:[~2023-04-28 12:32 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-28 12:32 [pve-devel] [PATCH-SERIES qemu-server/storage] improve RBD resize Fiona Ebner
2023-04-28 12:32 ` Fiona Ebner [this message]
2023-04-28 12:32 ` [pve-devel] [PATCH storage 1/2] rbd: don't specify allow-shrink flag Fiona Ebner
2023-04-28 12:32 ` [pve-devel] [PATCH storage 2/2] rbd: volume resize: avoid passing floating point value to rbd Fiona Ebner
2023-06-06 17:37   ` Thomas Lamprecht
2023-06-07  8:51     ` Fiona Ebner
2023-06-06 17:45 ` [pve-devel] applied-series: [PATCH-SERIES qemu-server/storage] improve RBD resize Thomas Lamprecht

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=20230428123209.124097-2-f.ebner@proxmox.com \
    --to=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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal