* [PATCH qemu-server v2 0/2] fix #7828: avoid parsing absolute paths as volume ids. @ 2026-07-22 9:52 Thomas Ellmenreich 2026-07-22 9:52 ` [PATCH qemu-server v2 1/2] refactor checks for absolute path as drive Thomas Ellmenreich 2026-07-22 9:52 ` [PATCH qemu-server v2 2/2] fix #7828: avoid parsing absolute paths as volume ids Thomas Ellmenreich 0 siblings, 2 replies; 9+ messages in thread From: Thomas Ellmenreich @ 2026-07-22 9:52 UTC (permalink / raw) To: pve-devel; +Cc: Thomas Ellmenreich This series refactors all checks of drive and volume ids as absolute paths into two separate helper functions, and uses them in all occurrences of such checks. The new helper functions are then also applied to make sure that drives that are absolute paths are not added as unused drives to a vm. Just like is the case for cdrom drives. This issue was reported in the community forum: [0]. The issue arose, as @Daniel Kral mentioned, because of the addition of the config check and parsing of the volume id with commit: 5b5c5768, which did not consider the possibility of absolute paths as volume ids. Changes since v1: - Prepended an additional patch to introduce the two helper functions, as the check for absolute paths was performed a considerable amount of times. - Use of the created helper functions: - Addition of the "Fixes 5b..." to the commit message [0]: https://forum.proxmox.com/threads/possible-regression-in-pve-9-2-x-unable-to-remove-physical-passthrough-disks-using-qm-set-delete.185191/ qemu-manager: Thomas Ellmenreich (2): refactor checks for absolute path as drive fix #7828: avoid parsing absolute paths as volume ids src/PVE/QemuConfig.pm | 2 +- src/PVE/QemuMigrate.pm | 2 +- src/PVE/QemuServer.pm | 22 +++++++++------------- src/PVE/QemuServer/Blockdev.pm | 4 ++-- src/PVE/QemuServer/Drive.pm | 8 +++++++- src/PVE/QemuServer/Helpers.pm | 5 +++++ 6 files changed, 25 insertions(+), 18 deletions(-) Summary over all repositories: 6 files changed, 25 insertions(+), 18 deletions(-) -- Generated by murpp 0.12.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH qemu-server v2 1/2] refactor checks for absolute path as drive 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 2026-07-22 12:45 ` Daniel Kral 2026-07-22 9:52 ` [PATCH qemu-server v2 2/2] fix #7828: avoid parsing absolute paths as volume ids Thomas Ellmenreich 1 sibling, 1 reply; 9+ messages in thread From: Thomas Ellmenreich @ 2026-07-22 9:52 UTC (permalink / raw) To: pve-devel; +Cc: Thomas Ellmenreich 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH qemu-server v2 1/2] refactor checks for absolute path as drive 2026-07-22 9:52 ` [PATCH qemu-server v2 1/2] refactor checks for absolute path as drive Thomas Ellmenreich @ 2026-07-22 12:45 ` Daniel Kral 2026-07-22 13:40 ` Thomas Ellmenreich 2026-07-22 14:01 ` Fiona Ebner 0 siblings, 2 replies; 9+ messages in thread From: Daniel Kral @ 2026-07-22 12:45 UTC (permalink / raw) To: Thomas Ellmenreich, pve-devel Thanks for the quick v2! On Wed Jul 22, 2026 at 11:52 AM CEST, Thomas Ellmenreich wrote: > 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(-) > [ snip ] > 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 nit: should be sorted alphabetically > 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|^/|; Hm, the !$volid check should be included in the check above as well as drive_is_absolute_path() doesn't check this and shouldn't need to. Though if there's good reason that !$volid will never be true here, this can be removed in a previous commit with a clear reasoning why. > - > 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}; same here > > 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|^/|; same here > - > 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); nit: should also be alphabetically sorted > 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 nit: should be sorted alphabetically > 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 { the subroutine name is rather short in description, this should be at least volid_is_absolute_path() or volumeid_is_absolute_path() or something different. > + my ($volid) = @_; > + return $volid =~ m|^/|; > +} > + > 1; ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH qemu-server v2 1/2] refactor checks for absolute path as drive 2026-07-22 12:45 ` Daniel Kral @ 2026-07-22 13:40 ` Thomas Ellmenreich 2026-07-22 14:01 ` Fiona Ebner 1 sibling, 0 replies; 9+ messages in thread From: Thomas Ellmenreich @ 2026-07-22 13:40 UTC (permalink / raw) To: Daniel Kral, pve-devel On Wed Jul 22, 2026 at 2:45 PM CEST, Daniel Kral wrote: [snip] > Hm, the !$volid check should be included in the check above as well as > drive_is_absolute_path() doesn't check this and shouldn't need to. > > Though if there's good reason that !$volid will never be true here, this > can be removed in a previous commit with a clear reasoning why. Good catch, was a thinking mistake on my part. Thought the `!volid` check was unnecessary because the regex cleanly handles it, although that misses the point. Will reintroduce it. [snip] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH qemu-server v2 1/2] refactor checks for absolute path as drive 2026-07-22 12:45 ` Daniel Kral 2026-07-22 13:40 ` Thomas Ellmenreich @ 2026-07-22 14:01 ` Fiona Ebner 1 sibling, 0 replies; 9+ messages in thread From: Fiona Ebner @ 2026-07-22 14:01 UTC (permalink / raw) To: Daniel Kral, Thomas Ellmenreich, pve-devel Am 22.07.26 um 2:45 PM schrieb Daniel Kral: > On Wed Jul 22, 2026 at 11:52 AM CEST, Thomas Ellmenreich wrote: >> 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. While these helpers are fine at a glance, a classify_drive_volid() similar to LXC/Config.pm's classify_mountpoint() might be a bit nicer in the long run. It could return 'absolute', 'none', 'cdrom' and 'volume'. >> @@ -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|^/|; > > Hm, the !$volid check should be included in the check above as well as > drive_is_absolute_path() doesn't check this and shouldn't need to. > > Though if there's good reason that !$volid will never be true here, this > can be removed in a previous commit with a clear reasoning why. We should be protected. We don't allow empty values as part of a property string and even if, the schema validation against 'pve-volume-id-or-qm-path' would fail. The implementation in foreach_volume_full() in pve-guest-common won't call the closure if parsing fails. So I think dropping the check is fine. If it really triggers, it will just result in a warning pointing to a programming error somewhere else, which we should fix then. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH qemu-server v2 2/2] fix #7828: avoid parsing absolute paths as volume ids 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 ` [PATCH qemu-server v2 1/2] refactor checks for absolute path as drive Thomas Ellmenreich @ 2026-07-22 9:52 ` Thomas Ellmenreich 2026-07-22 13:01 ` Fiona Ebner 2026-07-22 14:10 ` Elias Huhsovitz 1 sibling, 2 replies; 9+ messages in thread From: Thomas Ellmenreich @ 2026-07-22 9:52 UTC (permalink / raw) To: pve-devel; +Cc: Thomas Ellmenreich Just like in the case of cdroms, absolute paths to storage drives can be skipped when beeing marked as unused as they cannot be owned by the vm. Fixes: 5b5c5768 ("display warnings for storage errors or if storage no longer exists") Signed-off-by: Thomas Ellmenreich <t.ellmenreich@proxmox.com> --- src/PVE/QemuServer.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm index ce832c8e..ce0eda74 100644 --- a/src/PVE/QemuServer.pm +++ b/src/PVE/QemuServer.pm @@ -1575,7 +1575,7 @@ sub vmconfig_register_unused_drive { eval { PVE::Storage::vdisk_free($storecfg, $drive->{file}) }; warn $@ if $@; delete $conf->{'special-sections'}->{cloudinit}; - } elsif (!drive_is_cdrom($drive)) { + } elsif (!drive_is_cdrom($drive) && !drive_is_absolute_path($drive)) { my $volid = $drive->{file}; my ($storeid, undef) = PVE::Storage::parse_volume_id($volid); if (PVE::Storage::storage_config($storecfg, $storeid, 1)) { -- 2.47.3 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH qemu-server v2 2/2] fix #7828: avoid parsing absolute paths as volume ids 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 1 sibling, 1 reply; 9+ messages in thread From: Fiona Ebner @ 2026-07-22 13:01 UTC (permalink / raw) To: Thomas Ellmenreich, pve-devel Am 22.07.26 um 11:53 AM schrieb Thomas Ellmenreich: > Just like in the case of cdroms, absolute paths to storage drives can be > skipped when beeing marked as unused as they cannot be owned by the vm. > > Fixes: 5b5c5768 ("display warnings for storage errors or if storage no longer exists") > Signed-off-by: Thomas Ellmenreich <t.ellmenreich@proxmox.com> > --- > src/PVE/QemuServer.pm | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm > index ce832c8e..ce0eda74 100644 > --- a/src/PVE/QemuServer.pm > +++ b/src/PVE/QemuServer.pm > @@ -1575,7 +1575,7 @@ sub vmconfig_register_unused_drive { > eval { PVE::Storage::vdisk_free($storecfg, $drive->{file}) }; > warn $@ if $@; > delete $conf->{'special-sections'}->{cloudinit}; > - } elsif (!drive_is_cdrom($drive)) { > + } elsif (!drive_is_cdrom($drive) && !drive_is_absolute_path($drive)) { > my $volid = $drive->{file}; > my ($storeid, undef) = PVE::Storage::parse_volume_id($volid); To fix the regression, I would prefer to use the noerr variant of parse_volume_id() here, because that is what tells us whether the volume is PVE-managed or not. We already do that in some other places like import. And then continue only if we got a result for $storeid. > if (PVE::Storage::storage_config($storecfg, $storeid, 1)) { ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH qemu-server v2 2/2] fix #7828: avoid parsing absolute paths as volume ids 2026-07-22 13:01 ` Fiona Ebner @ 2026-07-22 13:11 ` Thomas Ellmenreich 0 siblings, 0 replies; 9+ messages in thread From: Thomas Ellmenreich @ 2026-07-22 13:11 UTC (permalink / raw) To: Fiona Ebner, pve-devel, d.kral > To fix the regression, I would prefer to use the noerr variant of > parse_volume_id() here, because that is what tells us whether the volume > is PVE-managed or not. We already do that in some other places like > import. And then continue only if we got a result for $storeid. That sounds like a better solution for me, what do you think @Daniel? ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH qemu-server v2 2/2] fix #7828: avoid parsing absolute paths as volume ids 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 14:10 ` Elias Huhsovitz 1 sibling, 0 replies; 9+ messages in thread From: Elias Huhsovitz @ 2026-07-22 14:10 UTC (permalink / raw) To: Thomas Ellmenreich, pve-devel Small typo, otherwise see review by fiona Ebner [1]. [1:] https://lore.proxmox.com/all/8abb29af-e16d-456b-ab14-cd423abdd801@proxmox.com/ On Wed Jul 22, 2026 at 11:52 AM CEST, Thomas Ellmenreich wrote: > Just like in the case of cdroms, absolute paths to storage drives can be > skipped when beeing marked as unused as they cannot be owned by the vm. ^^^ small typo > > Fixes: 5b5c5768 ("display warnings for storage errors or if storage no longer exists") > Signed-off-by: Thomas Ellmenreich <t.ellmenreich@proxmox.com> > --- > src/PVE/QemuServer.pm | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm > index ce832c8e..ce0eda74 100644 > --- a/src/PVE/QemuServer.pm > +++ b/src/PVE/QemuServer.pm > @@ -1575,7 +1575,7 @@ sub vmconfig_register_unused_drive { > eval { PVE::Storage::vdisk_free($storecfg, $drive->{file}) }; > warn $@ if $@; > delete $conf->{'special-sections'}->{cloudinit}; > - } elsif (!drive_is_cdrom($drive)) { > + } elsif (!drive_is_cdrom($drive) && !drive_is_absolute_path($drive)) { > my $volid = $drive->{file}; > my ($storeid, undef) = PVE::Storage::parse_volume_id($volid); > if (PVE::Storage::storage_config($storecfg, $storeid, 1)) { ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-07-22 14:10 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 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 ` [PATCH qemu-server v2 1/2] refactor checks for absolute path as drive Thomas Ellmenreich 2026-07-22 12:45 ` 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
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.