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 v2 3/3] blockdev: delete/replace: re-use detach() helper
Date: Tue, 12 Aug 2025 13:56:31 +0200	[thread overview]
Message-ID: <20250812115652.79330-4-f.ebner@proxmox.com> (raw)
In-Reply-To: <20250812115652.79330-1-f.ebner@proxmox.com>

Re-using the detach() helper has the side effect of avoiding logging
errors to syslog for automatically removed child nodes. This should be
the case for all file nodes here. None are explicitly added via
blockdev-add and thus QEMU already auto-removes them.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---

Changes in v2:
* use eval+warn around new detach() call sites, as failure should not
  be critical there.

 src/PVE/QemuServer/Blockdev.pm | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/src/PVE/QemuServer/Blockdev.pm b/src/PVE/QemuServer/Blockdev.pm
index 04eeed3c..f5798cbf 100644
--- a/src/PVE/QemuServer/Blockdev.pm
+++ b/src/PVE/QemuServer/Blockdev.pm
@@ -873,9 +873,8 @@ sub blockdev_external_snapshot {
 sub blockdev_delete {
     my ($storecfg, $vmid, $drive, $file_blockdev, $fmt_blockdev, $snap) = @_;
 
-    #add eval as reopen is auto removing the old nodename automatically only if it was created at vm start in command line argument
-    eval { mon_cmd($vmid, 'blockdev-del', 'node-name' => $fmt_blockdev->{'node-name'}) };
-    eval { mon_cmd($vmid, 'blockdev-del', 'node-name' => $file_blockdev->{'node-name'}) };
+    eval { detach($vmid, $fmt_blockdev->{'node-name'}); };
+    warn "detaching block node for $file_blockdev->{filename} failed - $@" if $@;
 
     #delete the file (don't use vdisk_free as we don't want to delete all snapshot chain)
     print "delete old $file_blockdev->{filename}\n";
@@ -982,9 +981,8 @@ sub blockdev_replace {
     }
 
     # delete old file|fmt nodes
-    # add eval as reopen is auto removing the old nodename automatically only if it was created at vm start in command line argument
-    eval { mon_cmd($vmid, 'blockdev-del', 'node-name' => $src_fmt_blockdev_name) };
-    eval { mon_cmd($vmid, 'blockdev-del', 'node-name' => $src_file_blockdev_name) };
+    eval { detach($vmid, $src_fmt_blockdev_name); };
+    warn "detaching block node for $src_snap failed - $@" if $@;
 }
 
 sub blockdev_commit {
-- 
2.47.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


  parent reply	other threads:[~2025-08-12 11:56 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-12 11:56 [pve-devel] [PATCH-SERIES qemu-server v2 0/3] qmp: improve error handling for mon_cmd() Fiona Ebner
2025-08-12 11:56 ` [pve-devel] [PATCH qemu-server v2 1/3] qmp client: add $noerr argument Fiona Ebner
2025-08-12 11:56 ` [pve-devel] [PATCH qemu-server v2 2/3] blockdev: attach/detach: silence errors for QMP commands for which failure may be expected Fiona Ebner
2025-08-12 11:56 ` Fiona Ebner [this message]
2025-08-12 12:51 ` [pve-devel] applied-series: [PATCH-SERIES qemu-server v2 0/3] qmp: improve error handling for mon_cmd() Fabian Grünbichler

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=20250812115652.79330-4-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