public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Ellmenreich <t.ellmenreich@proxmox.com>
To: pve-devel@lists.proxmox.com
Cc: Thomas Ellmenreich <t.ellmenreich@proxmox.com>
Subject: [PATCH qemu-server v2 1/2] refactor checks for absolute path as drive
Date: Wed, 22 Jul 2026 11:52:50 +0200	[thread overview]
Message-ID: <20260722095251.89606-2-t.ellmenreich@proxmox.com> (raw)
In-Reply-To: <20260722095251.89606-1-t.ellmenreich@proxmox.com>

Previously there were a bunch of separate checks for whether a drive was
an absolute path. These have been refactored to use two checks in
QemuServer::Drive and QemuServer::Helpers.

Signed-off-by: Thomas Ellmenreich <t.ellmenreich@proxmox.com>
---
 src/PVE/QemuConfig.pm          |  2 +-
 src/PVE/QemuMigrate.pm         |  2 +-
 src/PVE/QemuServer.pm          | 20 ++++++++------------
 src/PVE/QemuServer/Blockdev.pm |  4 ++--
 src/PVE/QemuServer/Drive.pm    |  8 +++++++-
 src/PVE/QemuServer/Helpers.pm  |  5 +++++
 6 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/src/PVE/QemuConfig.pm b/src/PVE/QemuConfig.pm
index 26f0fda2..d8733d9d 100644
--- a/src/PVE/QemuConfig.pm
+++ b/src/PVE/QemuConfig.pm
@@ -145,7 +145,7 @@ sub get_replicatable_volumes {
 
         return if !$cleanup && !$attr->{replicate};
 
-        if ($volid =~ m|^/|) {
+        if (PVE::QemuServer::Helpers::is_absolute_path($volid)) {
             return if !$attr->{replicate};
             return if $cleanup || $noerr;
             die "unable to replicate local file/device '$volid'\n";
diff --git a/src/PVE/QemuMigrate.pm b/src/PVE/QemuMigrate.pm
index 8da6f15d..dcf911a5 100644
--- a/src/PVE/QemuMigrate.pm
+++ b/src/PVE/QemuMigrate.pm
@@ -405,7 +405,7 @@ sub scan_local_volumes {
         my $test_volid = sub {
             my ($volid, $attr) = @_;
 
-            if ($volid =~ m|^/|) {
+            if (PVE::QemuServer::Helpers::is_absolute_path($volid)) {
                 return if $attr->{shared};
                 $local_volumes->{$volid}->{ref} = 'config';
                 die "local file/device\n";
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 191ae549..ce832c8e 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -75,6 +75,7 @@ use PVE::QemuServer::Drive qw(
     checked_volume_format
     drive_is_cloudinit
     drive_is_cdrom
+    drive_is_absolute_path
     parse_drive
     print_drive
     storage_allows_io_uring_default
@@ -913,7 +914,7 @@ PVE::JSONSchema::register_format(
 sub verify_volume_id_or_absolute_path {
     my ($volid, $noerr) = @_;
 
-    return $volid if $volid =~ m|^/|;
+    return $volid if PVE::QemuServer::Helpers::is_absolute_path($volid);
 
     $volid = eval { PVE::JSONSchema::check_format('pve-volume-id', $volid, '') };
     if ($@) {
@@ -1555,7 +1556,7 @@ sub print_vga_device {
 sub vm_is_volid_owner {
     my ($storecfg, $vmid, $volid) = @_;
 
-    if ($volid !~ m|^/|) {
+    if (!PVE::QemuServer::Helpers::is_absolute_path($volid)) {
         my ($path, $owner);
         eval { ($path, $owner) = PVE::Storage::path($storecfg, $volid); };
         log_warn("ownership of volume '$volid' could not be determined: $@") if $@;
@@ -1841,11 +1842,9 @@ sub destroy_vm {
             { include_unused => 1 },
             sub {
                 my ($ds, $drive) = @_;
-                return if drive_is_cdrom($drive);
+                return if drive_is_cdrom($drive) || drive_is_absolute_path($drive);
 
                 my $volid = $drive->{file};
-                return if !$volid || $volid =~ m|^/|;
-
                 my $result = eval { PVE::Storage::volume_is_base_and_used($storecfg, $volid) };
                 # early check, removal of volume will fail later anyway, so warning here is fine
                 log_warn("failed to check if volume '$volid' is used by linked clones: $@")
@@ -1859,10 +1858,9 @@ sub destroy_vm {
     my $volids = {};
     my $remove_owned_drive = sub {
         my ($ds, $drive) = @_;
-        return if drive_is_cdrom($drive, 1);
+        return if drive_is_cdrom($drive, 1) || drive_is_absolute_path($drive);
 
         my $volid = $drive->{file};
-        return if !$volid || $volid =~ m|^/|;
         return if $volids->{$volid};
 
         my ($path, $owner) = eval { PVE::Storage::path($storecfg, $volid) };
@@ -6085,7 +6083,7 @@ sub get_vm_volumes {
         sub {
             my ($volid, $attr) = @_;
 
-            return if $volid =~ m|^/|;
+            return if PVE::QemuServer::Helpers::is_absolute_path($volid);
 
             my ($sid, $volname) = PVE::Storage::parse_volume_id($volid, 1);
             return if !$sid;
@@ -6485,7 +6483,7 @@ sub tar_restore_cleanup {
             if ($line =~ m/vzdump:([^\s:]*):(\S+)$/) {
                 my $volid = $2;
                 eval {
-                    if ($volid =~ m|^/|) {
+                    if (PVE::QemuServer::Helpers::is_absolute_path($volid)) {
                         unlink $volid || die 'unlink failed\n';
                     } else {
                         PVE::Storage::vdisk_free($storecfg, $volid);
@@ -6530,11 +6528,9 @@ my $restore_cleanup_oldconf = sub {
         sub {
             my ($ds, $drive) = @_;
 
-            return if drive_is_cdrom($drive, 1);
+            return if drive_is_cdrom($drive, 1) || drive_is_absolute_path($drive);
 
             my $volid = $drive->{file};
-            return if !$volid || $volid =~ m|^/|;
-
             my ($path, $owner) = PVE::Storage::path($storecfg, $volid);
             return if !$path || !$owner || ($owner != $vmid);
 
diff --git a/src/PVE/QemuServer/Blockdev.pm b/src/PVE/QemuServer/Blockdev.pm
index 101c747c..78a53ff5 100644
--- a/src/PVE/QemuServer/Blockdev.pm
+++ b/src/PVE/QemuServer/Blockdev.pm
@@ -11,7 +11,7 @@ use JSON;
 use PVE::JSONSchema qw(json_bool);
 use PVE::Storage;
 
-use PVE::QemuServer::Drive qw(drive_is_cdrom);
+use PVE::QemuServer::Drive qw(drive_is_cdrom drive_is_absolute_path);
 use PVE::QemuServer::Helpers;
 use PVE::QemuServer::Machine;
 use PVE::QemuServer::Monitor qw(mon_cmd qmp_cmd qsd_qmp_peer vm_qmp_peer);
@@ -299,7 +299,7 @@ sub generate_file_blockdev {
     } elsif ($drive->{file} eq 'cdrom') {
         my $path = PVE::QemuServer::Drive::get_iso_path($storecfg, $drive->{file});
         $blockdev = { driver => 'host_cdrom', filename => "$path" };
-    } elsif ($drive->{file} =~ m|^/|) {
+    } elsif (drive_is_absolute_path($drive)) {
         my $path = $drive->{file};
         # The 'file' driver only works for regular files. The check below is taken from
         # block/file-posix.c:hdev_probe_device() in QEMU. To detect CD-ROM host devices, QEMU issues
diff --git a/src/PVE/QemuServer/Drive.pm b/src/PVE/QemuServer/Drive.pm
index b80b7dbb..35256acf 100644
--- a/src/PVE/QemuServer/Drive.pm
+++ b/src/PVE/QemuServer/Drive.pm
@@ -23,6 +23,7 @@ our @EXPORT_OK = qw(
     checked_volume_format
     drive_is_cloudinit
     drive_is_cdrom
+    drive_is_absolute_path
     parse_drive
     print_drive
     storage_allows_io_uring_default
@@ -766,6 +767,11 @@ sub drive_is_cdrom {
     return $drive && $drive->{media} && ($drive->{media} eq 'cdrom');
 }
 
+sub drive_is_absolute_path {
+    my ($drive) = @_;
+    return PVE::QemuServer::Helpers::is_absolute_path($drive->{file});
+}
+
 sub parse_drive_interface {
     my ($key) = @_;
 
@@ -1061,7 +1067,7 @@ sub get_scsi_device_type {
     if (drive_is_cdrom($drive) || drive_is_cloudinit($drive)) {
         $devicetype = 'cd';
     } else {
-        if ($drive->{file} =~ m|^/|) {
+        if (drive_is_absolute_path($drive)) {
             $path = $drive->{file};
             if (my $info = path_is_scsi($path)) {
                 if ($info->{type} == 0 && $drive->{scsiblock}) {
diff --git a/src/PVE/QemuServer/Helpers.pm b/src/PVE/QemuServer/Helpers.pm
index dd17eef5..5cc3e8b5 100644
--- a/src/PVE/QemuServer/Helpers.pm
+++ b/src/PVE/QemuServer/Helpers.pm
@@ -389,4 +389,9 @@ sub get_host_phys_address_bits {
     return; # undef, cannot really do anything..
 }
 
+sub is_absolute_path {
+    my ($volid) = @_;
+    return $volid =~ m|^/|;
+}
+
 1;
-- 
2.47.3





  reply	other threads:[~2026-07-22  9:53 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-22  9:52 [PATCH qemu-server v2 0/2] fix #7828: avoid parsing absolute paths as volume ids Thomas Ellmenreich
2026-07-22  9:52 ` Thomas Ellmenreich [this message]
2026-07-22 12:45   ` [PATCH qemu-server v2 1/2] refactor checks for absolute path as drive Daniel Kral
2026-07-22 13:40     ` Thomas Ellmenreich
2026-07-22 14:01     ` Fiona Ebner
2026-07-22  9:52 ` [PATCH qemu-server v2 2/2] fix #7828: avoid parsing absolute paths as volume ids Thomas Ellmenreich
2026-07-22 13:01   ` Fiona Ebner
2026-07-22 13:11     ` Thomas Ellmenreich
2026-07-22 14:10   ` Elias Huhsovitz

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=20260722095251.89606-2-t.ellmenreich@proxmox.com \
    --to=t.ellmenreich@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