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
next prev 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox