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 2/3] add new classify_drive_file utility
Date: Mon,  3 Aug 2026 14:12:58 +0200	[thread overview]
Message-ID: <20260803121300.138287-3-t.ellmenreich@proxmox.com> (raw)
In-Reply-To: <20260803121300.138287-1-t.ellmenreich@proxmox.com>

To more easily and clearly branch off different types of drive files a
new utility function is introduced that classifies such a drive file.
The return options are 'none', 'cdrom', 'absolute' and 'volume'. In any
other case the function returns undef.

Signed-off-by: Thomas Ellmenreich <t.ellmenreich@proxmox.com>
---
 src/PVE/QemuServer.pm       | 29 ++++++++++++++++++-----------
 src/PVE/QemuServer/Drive.pm | 23 +++++++++++++++++++++++
 2 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index fdef20dc..0e998cbe 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -43,7 +43,6 @@ use PVE::PBSClient;
 use PVE::RESTEnvironment qw(log_warn);
 use PVE::RPCEnvironment;
 use PVE::SafeSyslog;
-use PVE::Storage;
 use PVE::SysFSTools;
 use PVE::Systemd;
 use PVE::Tools qw(run_command file_read_firstline file_get_contents dir_glob_foreach $IPV6RE);
@@ -73,6 +72,8 @@ use PVE::QemuServer::CPUFlags;
 use PVE::QemuServer::Drive qw(
     is_valid_drivename
     checked_volume_format
+    classify_drive_file
+    drive_has_absolute_path
     drive_is_cloudinit
     drive_is_cdrom
     parse_drive
@@ -1102,7 +1103,10 @@ sub cleanup_drive_path {
         $drive->{file} = $volid;
     }
 
-    $drive->{media} = 'cdrom' if !$drive->{media} && $drive->{file} =~ m/^(cdrom|none)$/;
+    my $file_type = classify_drive_file($drive->{file});
+
+    $drive->{media} = 'cdrom'
+        if !$drive->{media} && ($file_type eq "cdrom" || $file_type eq "none");
 }
 
 sub parse_hotplug_features {
@@ -1527,7 +1531,7 @@ sub print_vga_device {
 sub vm_is_volid_owner {
     my ($storecfg, $vmid, $volid) = @_;
 
-    if ($volid !~ m|^/|) {
+    if (classify_drive_file($volid) ne "absolute") {
         my ($path, $owner);
         eval { ($path, $owner) = PVE::Storage::path($storecfg, $volid); };
         log_warn("ownership of volume '$volid' could not be determined: $@") if $@;
@@ -1817,7 +1821,7 @@ sub destroy_vm {
                 return if drive_is_cdrom($drive);
 
                 my $volid = $drive->{file};
-                return if !$volid || $volid =~ m|^/|;
+                return if !$volid || drive_has_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
@@ -1835,7 +1839,7 @@ sub destroy_vm {
         return if drive_is_cdrom($drive, 1);
 
         my $volid = $drive->{file};
-        return if !$volid || $volid =~ m|^/|;
+        return if !$volid || drive_has_absolute_path($drive);
         return if $volids->{$volid};
 
         my ($path, $owner) = eval { PVE::Storage::path($storecfg, $volid) };
@@ -3597,7 +3601,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 (classify_drive_file($drive->{file}) ne "none") {
                     my $throttle_group =
                         PVE::QemuServer::Blockdev::generate_throttle_group($drive);
                     push @$cmd, '-object', to_json($throttle_group, { canonical => 1 });
@@ -5276,7 +5280,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 (
+                classify_drive_file($drive->{file}) eq "none"
+                && drive_is_cloudinit($old_drive)
+            ) {
                 vmconfig_register_unused_drive($storecfg, $vmid, $conf, $old_drive);
             }
 
@@ -6058,7 +6065,7 @@ sub get_vm_volumes {
         sub {
             my ($volid, $attr) = @_;
 
-            return if $volid =~ m|^/|;
+            return if classify_drive_file($volid) eq "absolute";
 
             my ($sid, $volname) = PVE::Storage::parse_volume_id($volid, 1);
             return if !$sid;
@@ -6082,7 +6089,7 @@ sub get_current_vm_volumes {
         sub {
             my ($ds, $drive) = @_;
 
-            if (PVE::Storage::parse_volume_id($drive->{file}, 1)) {
+            if (classify_drive_file($drive->{file}) eq "volume") {
                 check_volume_storage_type($storecfg, $drive->{file});
                 push $volumes->@*, $drive->{file};
             }
@@ -6458,7 +6465,7 @@ sub tar_restore_cleanup {
             if ($line =~ m/vzdump:([^\s:]*):(\S+)$/) {
                 my $volid = $2;
                 eval {
-                    if ($volid =~ m|^/|) {
+                    if (classify_drive_file($volid) eq "absolute") {
                         unlink $volid || die 'unlink failed\n';
                     } else {
                         PVE::Storage::vdisk_free($storecfg, $volid);
@@ -6506,7 +6513,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_has_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 52e4a89b..f2106141 100644
--- a/src/PVE/QemuServer/Drive.pm
+++ b/src/PVE/QemuServer/Drive.pm
@@ -21,6 +21,8 @@ our @EXPORT_OK = qw(
     is_valid_drivename
     checked_parse_volname
     checked_volume_format
+    classify_drive_file
+    drive_has_absolute_path
     drive_is_cloudinit
     drive_is_cdrom
     parse_drive
@@ -781,6 +783,22 @@ sub verify_volume_id_or_absolute_path {
     return $volid;
 }
 
+# Tries to classify the drive file as either 'none', 'cdrom', 'absolute'
+# or 'volume'. In all other cases it will return undef.
+sub classify_drive_file {
+    my ($volid) = @_;
+
+    if ($volid eq "none") {
+        return "none";
+    } elsif ($volid eq 'cdrom') {
+        return "cdrom";
+    } elsif ($volid =~ m|^/|) {
+        return "absolute";
+    } elsif (PVE::Storage::parse_volume_id($volid, 1)) {
+        return "volume";
+    }
+}
+
 sub drive_is_cloudinit {
     my ($drive) = @_;
     return $drive->{file} =~ m@[:/](?:vm-\d+-)?cloudinit(?:\.$QEMU_FORMAT_RE)?$@;
@@ -794,6 +812,11 @@ sub drive_is_cdrom {
     return $drive && $drive->{media} && ($drive->{media} eq 'cdrom');
 }
 
+sub drive_has_absolute_path {
+    my ($drive) = @_;
+    return classify_drive_file($drive->{file}) eq "absolute";
+}
+
 sub parse_drive_interface {
     my ($key) = @_;
 
-- 
2.47.3





  parent reply	other threads:[~2026-08-03 12:13 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-03 12:12 [PATCH qemu-server 0/3] refactor of volume_id classification Thomas Ellmenreich
2026-08-03 12:12 ` [PATCH qemu-server 1/3] moved verify_volume_id_or_* format registrations Thomas Ellmenreich
2026-08-04 14:09   ` Elias Huhsovitz
2026-08-03 12:12 ` Thomas Ellmenreich [this message]
2026-08-04 14:23   ` [PATCH qemu-server 2/3] add new classify_drive_file utility Elias Huhsovitz
2026-08-03 12:12 ` [PATCH qemu-server 3/3] removed some unnecessary undef checks Thomas Ellmenreich
2026-08-05  8:24   ` Elias Huhsovitz
2026-08-05  9:55     ` Fiona Ebner
2026-08-05  8:22 ` superseded: [PATCH qemu-server 0/3] refactor of volume_id classification Thomas Ellmenreich

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=20260803121300.138287-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 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