all lists on 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 2/2] use new classify_volume_id subroutine
Date: Tue, 28 Jul 2026 14:22:18 +0200	[thread overview]
Message-ID: <20260728122218.202963-3-t.ellmenreich@proxmox.com> (raw)
In-Reply-To: <20260728122218.202963-1-t.ellmenreich@proxmox.com>

refactors checks of volume ids for different patterns to use the new
classify_volume_id subroutine exposed by pve_storage.

Signed-off-by: Thomas Ellmenreich <t.ellmenreich@proxmox.com>
---
 src/PVE/QemuServer.pm       | 39 ++++++++++++++++++++++++-------------
 src/PVE/QemuServer/Drive.pm |  6 ++++++
 2 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 9aec7f9c..33176ab5 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -43,7 +43,12 @@ use PVE::PBSClient;
 use PVE::RESTEnvironment qw(log_warn);
 use PVE::RPCEnvironment;
 use PVE::SafeSyslog;
-use PVE::Storage;
+use PVE::Storage qw(
+    $VOL_ABSOLUTE
+    $VOL_CDROM
+    $VOL_NONE
+    $VOL_VOLUME
+);
 use PVE::SysFSTools;
 use PVE::Systemd;
 use PVE::Tools qw(run_command file_read_firstline file_get_contents dir_glob_foreach $IPV6RE);
@@ -73,6 +78,7 @@ use PVE::QemuServer::CPUFlags;
 use PVE::QemuServer::Drive qw(
     is_valid_drivename
     checked_volume_format
+    drive_is_absolute_path
     drive_is_cloudinit
     drive_is_cdrom
     parse_drive
@@ -900,7 +906,8 @@ PVE::JSONSchema::register_format('pve-volume-id-or-qm-path', \&verify_volume_id_
 sub verify_volume_id_or_qm_path {
     my ($volid, $noerr) = @_;
 
-    return $volid if $volid eq 'none' || $volid eq 'cdrom';
+    my $vol_type = PVE::Storage::classify_volume_id($volid, 1);
+    return $volid if $vol_type eq $VOL_NONE || $vol_type eq $VOL_CDROM;
 
     return verify_volume_id_or_absolute_path($volid, $noerr);
 }
@@ -913,7 +920,7 @@ PVE::JSONSchema::register_format(
 sub verify_volume_id_or_absolute_path {
     my ($volid, $noerr) = @_;
 
-    return $volid if $volid =~ m|^/|;
+    return $volid if PVE::Storage::classify_volume_id($volid, 1) eq $VOL_ABSOLUTE;
 
     $volid = eval { PVE::JSONSchema::check_format('pve-volume-id', $volid, '') };
     if ($@) {
@@ -1130,7 +1137,10 @@ sub cleanup_drive_path {
         $drive->{file} = $volid;
     }
 
-    $drive->{media} = 'cdrom' if !$drive->{media} && $drive->{file} =~ m/^(cdrom|none)$/;
+    my $vol_type = PVE::Storage::classify_volume_id($drive->{file}, 1);
+
+    $drive->{media} = 'cdrom'
+        if !$drive->{media} && ($vol_type eq $VOL_CDROM || $vol_type eq $VOL_NONE);
 }
 
 sub parse_hotplug_features {
@@ -1555,7 +1565,7 @@ sub print_vga_device {
 sub vm_is_volid_owner {
     my ($storecfg, $vmid, $volid) = @_;
 
-    if ($volid !~ m|^/|) {
+    if (PVE::Storage::classify_volume_id($volid, 1) ne $VOL_ABSOLUTE) {
         my ($path, $owner);
         eval { ($path, $owner) = PVE::Storage::path($storecfg, $volid); };
         log_warn("ownership of volume '$volid' could not be determined: $@") if $@;
@@ -1845,7 +1855,7 @@ sub destroy_vm {
                 return if drive_is_cdrom($drive);
 
                 my $volid = $drive->{file};
-                return if !$volid || $volid =~ m|^/|;
+                return if !$volid || drive_is_absolute_path($drive);
 
                 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
@@ -1863,7 +1873,7 @@ sub destroy_vm {
         return if drive_is_cdrom($drive, 1);
 
         my $volid = $drive->{file};
-        return if !$volid || $volid =~ m|^/|;
+        return if !$volid || drive_is_absolute_path($drive);
         return if $volids->{$volid};
 
         my ($path, $owner) = eval { PVE::Storage::path($storecfg, $volid) };
@@ -3625,7 +3635,7 @@ sub config_to_command {
             my $live_restore = $live_restore_backing->{$ds};
 
             if (min_version($machine_version, 10, 0)) { # for the switch to -blockdev
-                if ($drive->{file} ne 'none') {
+                if (PVE::Storage::classify_volume_id($drive->{file}, 1) ne $VOL_NONE) {
                     my $throttle_group =
                         PVE::QemuServer::Blockdev::generate_throttle_group($drive);
                     push @$cmd, '-object', to_json($throttle_group, { canonical => 1 });
@@ -5304,7 +5314,10 @@ sub vmconfig_update_disk {
             eval { PVE::QemuServer::Blockdev::change_medium($storecfg, $vmid, $opt, $drive); };
             my $err = $@;
 
-            if ($drive->{file} eq 'none' && drive_is_cloudinit($old_drive)) {
+            if (
+                PVE::Storage::classify_volume_id($drive->{file}, 1) eq $VOL_NONE
+                && drive_is_cloudinit($old_drive)
+            ) {
                 vmconfig_register_unused_drive($storecfg, $vmid, $conf, $old_drive);
             }
 
@@ -6086,7 +6099,7 @@ sub get_vm_volumes {
         sub {
             my ($volid, $attr) = @_;
 
-            return if $volid =~ m|^/|;
+            return if PVE::Storage::classify_volume_id($volid, 1) eq $VOL_ABSOLUTE;
 
             my ($sid, $volname) = PVE::Storage::parse_volume_id($volid, 1);
             return if !$sid;
@@ -6110,7 +6123,7 @@ sub get_current_vm_volumes {
         sub {
             my ($ds, $drive) = @_;
 
-            if (PVE::Storage::parse_volume_id($drive->{file}, 1)) {
+            if (PVE::Storage::classify_volume_id($drive->{file}, 1) eq $VOL_VOLUME) {
                 check_volume_storage_type($storecfg, $drive->{file});
                 push $volumes->@*, $drive->{file};
             }
@@ -6486,7 +6499,7 @@ sub tar_restore_cleanup {
             if ($line =~ m/vzdump:([^\s:]*):(\S+)$/) {
                 my $volid = $2;
                 eval {
-                    if ($volid =~ m|^/|) {
+                    if (PVE::Storage::classify_volume_id($volid, 1) eq $VOL_ABSOLUTE) {
                         unlink $volid || die 'unlink failed\n';
                     } else {
                         PVE::Storage::vdisk_free($storecfg, $volid);
@@ -6534,7 +6547,7 @@ my $restore_cleanup_oldconf = sub {
             return if drive_is_cdrom($drive, 1);
 
             my $volid = $drive->{file};
-            return if !$volid || $volid =~ m|^/|;
+            return if !$volid || drive_is_absolute_path($drive);
 
             my ($path, $owner) = PVE::Storage::path($storecfg, $volid);
             return if !$path || !$owner || ($owner != $vmid);
diff --git a/src/PVE/QemuServer/Drive.pm b/src/PVE/QemuServer/Drive.pm
index b80b7dbb..5738c0bc 100644
--- a/src/PVE/QemuServer/Drive.pm
+++ b/src/PVE/QemuServer/Drive.pm
@@ -21,6 +21,7 @@ our @EXPORT_OK = qw(
     is_valid_drivename
     checked_parse_volname
     checked_volume_format
+    drive_is_absolute_path
     drive_is_cloudinit
     drive_is_cdrom
     parse_drive
@@ -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::Storage::classify_volume_id($drive->{file}, 1) eq $PVE::Storage::VOL_ABSOLUTE;
+}
+
 sub parse_drive_interface {
     my ($key) = @_;
 
-- 
2.47.3





  parent reply	other threads:[~2026-07-28 12:22 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-28 12:22 [RFC qemu-server/storage 0/2] refactor of volume_id classification Thomas Ellmenreich
2026-07-28 12:22 ` [PATCH storage 1/2] add subroutine to classify volume ids Thomas Ellmenreich
2026-07-31 10:13   ` Fiona Ebner
2026-07-31 10:21     ` Fiona Ebner
2026-07-28 12:22 ` Thomas Ellmenreich [this message]
2026-07-31 10:13   ` [PATCH qemu-server 2/2] use new classify_volume_id subroutine Fiona Ebner
2026-07-28 13:30 ` [RFC qemu-server/storage 0/2] refactor of volume_id classification Max R. Carrara
2026-07-28 14:29   ` Thomas Ellmenreich
2026-07-29  8:27     ` Max R. Carrara
2026-07-31  9:56     ` Fiona Ebner

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=20260728122218.202963-3-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 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