[pve-devel] [PATCH container/qemu-server/storage v8 00/10] fix #3711 and adapt drive detach/remove behavior

[pve-devel] [PATCH container/qemu-server/storage v8 00/10] fix #3711 and adapt drive detach/remove behavior

[Michael Köppl]

This series aims to fix #3711 [0] and streamline the detach/remove
behavior around volumes that are either mounted into a container or
attached to a VM as a hard disk. It also contains adds warnings in case
a volume's underlying storage does not exist anymore. It is a
continuation of a series from 2022 [1], but makes the following changes:

Changes v7 -> v8:
- Rebased on master

Changes v6 -> v7:
- Added descriptions of changes made to original patches where
  applicable
- Move check if storage exists to vdisk_free() and parse_volname() in
  pve-storage, print warnings
- Remove explicit storage exist check in destroy_lxc_container, wrap in
  eval instead
- Move parse_volume() calls to where input is known to be volume
- Add patch that renames $volume to $volid in destroy_lxc_container()
  and delete_mountpoint_volume()
- Rewrite commit message of pve-container 3/4 (2/4 in v6) such that it's clear this
  is about future-proofing
- Add FIXME note regarding is_volume_in_use() check
- Add warning to destroy_vm() if volume is still used by linked clone
- Adapt commit message of qemu-server 2/4
- Add patch to use log_warn in destroy_vm() instead of warn
- Rewrite patch in in qemu-server to print warnings of storage does not
  exist instead of marking volume without underlying storage as unused
  (vmconfig_register_unused_drive())

Thanks to Fiona for the comprehensive feedback on v6.

Changes v5 -> v6:
- Fix links in cover letter
- Use Originally-by instead of Co-authored-by
- Add documentation for the second patch regarding the use of
  $mp->{volume} instead of $conf->{$opt}
- Note that no functional changes are intended for the the second patch

Changes v4 -> v5:
- Always ignore errors that originate from a removed storage and
  continue with destruction of a container or removal of a volume,
  instead of adding an option to ignore these errors.
- Remove web UI checkbox
- Remove formatting patch
- Additionally allow removing a mount point with a removed storage
  from a running container (previously hotplug removal was not possible)
- Fix style nits from v4 review
- Print warnings for any errors that occur instead of ignoring them
- Add explicit check if storage still exists when destroying a container
  to differentiate between that case and other error cases (which should
  still fail)

The first change also means that this patch series should be held back
until PVE 9. In the original patch series the option to ignore these
errors was made to avoid breaking existing behavior. After some off-list
discussion it seems more reasonable to avoid an additional option and
instead allow users to remove containers with mount points even if the
underlying storage of the mount point was removed.

Where at least some of the implementation was taken from the previous
series, the patch was marked to be Originally-by by the original author.

[0] https://bugzilla.proxmox.com/show_bug.cgi?id=3711
[1] https://lore.proxmox.com/pve-devel/20221125144008.2988072-1-s.hrdlicka@proxmox.com/t/#

pve-storage:

Michael Köppl (2):
  vdisk_free: print warning if underlying storage does not exist
  parse_volname: print warning if storage does not exists

 src/PVE/Storage.pm | 11 +++++++++++
 1 file changed, 11 insertions(+)


pve-container:

Michael Köppl (4):
  fix #3711: warn about storage errors during mountpoint delete
  destroy_lxc, delete_mp_volume: rename $volume to $volid
  config: ensure valid volid through parse_volume()
  add linked clone check when destroying container template

 src/PVE/LXC.pm        | 31 ++++++++++++++++++++++---------
 src/PVE/LXC/Config.pm | 11 +++++++----
 2 files changed, 29 insertions(+), 13 deletions(-)


qemu-server:

Michael Köppl (4):
  adapt linked clone check to not die if an error occurs during check
  fix #3711: make removal of VM possible if store does not exist anymore
  destroy_vm: use log_warn for vdisk_free errors for consistency
  display warnings for storage errors or if storage no longer exists

 src/PVE/QemuServer.pm | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)


Summary over all repositories:
  4 files changed, 56 insertions(+), 20 deletions(-)

-- 
Generated by git-murpp 0.8.0


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH storage v8 1/2] vdisk_free: print warning if underlying storage does not exist

[Michael Köppl]

Instead of failing later in the function, users are warned if the
underlying storage no longer exists.

Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
 src/PVE/Storage.pm | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index 69eb435f..ec875fd0 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -1082,6 +1082,12 @@ sub vdisk_free {
 
     my ($storeid, $volname) = parse_volume_id($volid);
     my $scfg = storage_config($cfg, $storeid);
+    if (!$scfg) {
+        log_warn(
+            "storage '$storeid' does not exist, mountpoint volume '$volid' could not be deleted");
+        return;
+    }
+
     my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
 
     activate_storage($cfg, $storeid);
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH storage v8 2/2] parse_volname: print warning if storage does not exists

[Michael Köppl]

To keep behavior around non-existent storages consistent with
vdisk_free(), also print warning if storage does not exist.

Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
 src/PVE/Storage.pm | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/PVE/Storage.pm b/src/PVE/Storage.pm
index ec875fd0..ddb8822e 100755
--- a/src/PVE/Storage.pm
+++ b/src/PVE/Storage.pm
@@ -529,6 +529,11 @@ sub parse_volname {
     my ($storeid, $volname) = parse_volume_id($volid);
 
     my $scfg = storage_config($cfg, $storeid);
+    if (!$scfg) {
+        log_warn(
+            "storage '$storeid' does not exist, mountpoint volume '$volid' could not be deleted");
+        return;
+    }
 
     my $plugin = PVE::Storage::Plugin->lookup($scfg->{type});
 
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH container v8 1/4] fix #3711: warn about storage errors during mountpoint delete

[Michael Köppl]

Errors during deletion of a mountpoint volume should not stop users from
destroying a container. Instead of failing, a warning is printed and the
destruction of the container continues.

Originally-by: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
 [ MK: remove ignore-storage-errors param ]
Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
 src/PVE/LXC.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index 02dd3164..dacd6cf0 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -970,7 +970,8 @@ sub destroy_lxc_container {
         return if $volids->{$volume};
         $volids->{$volume} = 1;
 
-        delete_mountpoint_volume($storage_cfg, $vmid, $volume);
+        eval { delete_mountpoint_volume($storage_cfg, $vmid, $volume); };
+        PVE::RESTEnvironment::log_warn("failed to delete mountpoint volume $volume: $@") if $@;
     };
     PVE::LXC::Config->foreach_volume_full($conf, { include_unused => 1 }, $remove_volume);
 
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH container v8 2/4] destroy_lxc, delete_mp_volume: rename $volume to $volid

[Michael Köppl]

$volid states more clearly that it's a volume ID, avoiding confusion
about the values these variables hold.

No functional change intended.

Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
 src/PVE/LXC.pm | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index dacd6cf0..2bb75dd8 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -945,16 +945,16 @@ sub get_primary_ips {
 }
 
 sub delete_mountpoint_volume {
-    my ($storage_cfg, $vmid, $volume) = @_;
+    my ($storage_cfg, $vmid, $volid) = @_;
 
-    return if PVE::LXC::Config->classify_mountpoint($volume) ne 'volume';
+    return if PVE::LXC::Config->classify_mountpoint($volid) ne 'volume';
 
-    my ($vtype, $name, $owner) = PVE::Storage::parse_volname($storage_cfg, $volume);
+    my ($vtype, $name, $owner) = PVE::Storage::parse_volname($storage_cfg, $volid);
 
     if ($vmid == $owner) {
-        PVE::Storage::vdisk_free($storage_cfg, $volume);
+        PVE::Storage::vdisk_free($storage_cfg, $volid);
     } else {
-        warn "ignore deletion of '$volume', CT $vmid isn't the owner!\n";
+        warn "ignore deletion of '$volid', CT $vmid isn't the owner!\n";
     }
 }
 
@@ -965,13 +965,13 @@ sub destroy_lxc_container {
     my $remove_volume = sub {
         my ($ms, $mountpoint) = @_;
 
-        my $volume = $mountpoint->{volume};
+        my $volid = $mountpoint->{volume};
 
-        return if $volids->{$volume};
-        $volids->{$volume} = 1;
+        return if $volids->{$volid};
+        $volids->{$volid} = 1;
 
-        eval { delete_mountpoint_volume($storage_cfg, $vmid, $volume); };
-        PVE::RESTEnvironment::log_warn("failed to delete mountpoint volume $volume: $@") if $@;
+        eval { delete_mountpoint_volume($storage_cfg, $vmid, $volid); };
+        PVE::RESTEnvironment::log_warn("failed to delete mountpoint volume $volid: $@") if $@;
     };
     PVE::LXC::Config->foreach_volume_full($conf, { include_unused => 1 }, $remove_volume);
 
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH container v8 3/4] config: ensure valid volid through parse_volume()

[Michael Köppl]

Even though the value in $conf->{$opt} contains a volume ID for unused
mount points at the moment, this is not guaranteed to be true in the
future. To ensure that a valid volume ID is used here, run call
parse_volume() first.

No functional change is intended here as the values of $conf->{$opt}
and $mp->{volume} are identical for unused mount points at the moment.

Also add FIXME for the is_volume_in_use() check for the branch that
handles mountpoint volumes that are in use. Currently, the check only
works by accident. The value in $conf->{$opt} isn’t just a volume ID -
it contains other things as well (such as mp=<path>). However, the
check only considers 'rootfs' and entries like 'mp0', 'mp1', etc. as
valid volume IDs. As a result, the volume is not detected to be in use.
Using a valid volume id with the current implementation, the check would
always report that the volume is in use and the mountpoint volume would
not be deleted. The correct behavior should use a valid volume id to
ensure that the volume can be safely removed.

Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
 src/PVE/LXC/Config.pm | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/src/PVE/LXC/Config.pm b/src/PVE/LXC/Config.pm
index 7bdb8b92..67f79a5a 100644
--- a/src/PVE/LXC/Config.pm
+++ b/src/PVE/LXC/Config.pm
@@ -1545,8 +1545,9 @@ sub vmconfig_hotplug_pending {
             if ($LXC_FASTPLUG_OPTIONS->{$opt}) {
                 # pass
             } elsif ($opt =~ m/^unused(\d+)$/) {
-                PVE::LXC::delete_mountpoint_volume($storecfg, $vmid, $conf->{$opt})
-                    if !$class->is_volume_in_use($conf, $conf->{$opt}, 1, 1);
+                my $volid = $class->parse_volume($opt, $conf->{$opt})->{volume};
+                PVE::LXC::delete_mountpoint_volume($storecfg, $vmid, $volid)
+                    if !$class->is_volume_in_use($conf, $volid, 1, 1);
             } elsif ($opt eq 'swap') {
                 $hotplug_memory->(undef, 0);
             } elsif ($opt eq 'cpulimit') {
@@ -1638,12 +1639,14 @@ sub vmconfig_apply_pending {
             if ($opt =~ m/^mp(\d+)$/) {
                 my $mp = $class->parse_volume($opt, $conf->{$opt});
                 if ($mp->{type} eq 'volume') {
+                    # FIXME: use $mp->{volume} for is_volume_in_use, fix conditions for check
                     $class->add_unused_volume($conf, $mp->{volume})
                         if !$class->is_volume_in_use($conf, $conf->{$opt}, 1, 1);
                 }
             } elsif ($opt =~ m/^unused(\d+)$/) {
-                PVE::LXC::delete_mountpoint_volume($storecfg, $vmid, $conf->{$opt})
-                    if !$class->is_volume_in_use($conf, $conf->{$opt}, 1, 1);
+                my $volid = $class->parse_volume($opt, $conf->{$opt})->{volume};
+                PVE::LXC::delete_mountpoint_volume($storecfg, $vmid, $volid)
+                    if !$class->is_volume_in_use($conf, $volid, 1, 1);
             } elsif ($opt =~ m/^net(\d+)$/) {
                 if ($have_sdn) {
                     my $net = $class->parse_lxc_network($conf->{$opt});
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH container v8 4/4] add linked clone check when destroying container template

[Michael Köppl]

This check matches the behavior already implemented for VMs and prevents
partial storage deletion if a container template has a linked clone. In
such cases, the destruction of the container template will now fail,
informing the user that the base volume is still in use by the linked
clone. In case of a storage error (such as the underlying storage no
existing anymore), a warning will be printed and execution continues,
mimicking the handling of storage errors in later stages of
destroy_lxc_container().

Originally-by: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
 [ MK: use classify_mountpoint instead of pattern matching
       display warning for storage errors during linked clone check
       resolve style nit ]
Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
 src/PVE/LXC.pm | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/src/PVE/LXC.pm b/src/PVE/LXC.pm
index 2bb75dd8..977cf6c8 100644
--- a/src/PVE/LXC.pm
+++ b/src/PVE/LXC.pm
@@ -962,6 +962,25 @@ sub destroy_lxc_container {
     my ($storage_cfg, $vmid, $conf, $replacement_conf, $purge_unreferenced) = @_;
 
     my $volids = {};
+
+    if ($conf->{template}) {
+        PVE::LXC::Config->foreach_volume_full(
+            $conf,
+            { include_unused => 1 },
+            sub {
+                my ($ms, $mountpoint) = @_;
+                my $volid = $mountpoint->{volume};
+                return if !$volid || PVE::LXC::Config->classify_mountpoint($volid) ne 'volume';
+                my $result =
+                    eval { PVE::Storage::volume_is_base_and_used($storage_cfg, $volid) };
+                PVE::RESTEnvironment::log_warn(
+                    "failed to check if volume '$volid' is used by linked clones: $@")
+                    if $@;
+                die "base volume '$volid' is still in use by linked clone\n" if $result;
+            },
+        );
+    }
+
     my $remove_volume = sub {
         my ($ms, $mountpoint) = @_;
 
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH qemu-server v8 1/4] adapt linked clone check to not die if an error occurs during check

[Michael Köppl]

Align error handling behavior when checking for linked clones with the
rest of destroy_vm's error handling approach. In case an error occurred,
a warning is printed and the execution continues, since:

1. The same validation occurs later in the process
2. The VM removal will still be blocked if the volume has linked clones

Originally-by: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
 [ MK: log_warn if check fails instead of ignoring
       resolve style nits ]
Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
 src/PVE/QemuServer.pm | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index e7c98520..365e1183 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -1897,8 +1897,10 @@ sub destroy_vm {
                 my $volid = $drive->{file};
                 return if !$volid || $volid =~ m|^/|;
 
-                die "base volume '$volid' is still in use by linked cloned\n"
-                    if PVE::Storage::volume_is_base_and_used($storecfg, $volid);
+                my $result = eval { PVE::Storage::volume_is_base_and_used($storecfg, $volid) };
+                log_warn("failed to check if volume '$volid' is used by linked clones: $@")
+                    if $@;
+                die "base volume '$volid' is still in use by linked cloned\n" if $result;
 
             },
         );
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH qemu-server v8 2/4] fix #3711: make removal of VM possible if store does not exist anymore

[Michael Köppl]

Similar to the handling of storage errors in other parts of
destroy_vm(), an error during the call to PVE::Storage::path() should
not stop the VM from being destroyed. Instead, the user should be warned
and the function should continue.

Originally-by: Stefan Hrdlicka <s.hrdlicka@proxmox.com>
 [ MK: log_warn if check fails instead of ignoring error ]
Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
 src/PVE/QemuServer.pm | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 365e1183..91b0d1e0 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -1915,7 +1915,8 @@ sub destroy_vm {
         return if !$volid || $volid =~ m|^/|;
         return if $volids->{$volid};
 
-        my ($path, $owner) = PVE::Storage::path($storecfg, $volid);
+        my ($path, $owner) = eval { PVE::Storage::path($storecfg, $volid) };
+        log_warn("failed to get path and owner of volume '$volid': $@") if $@;
         return if !$path || !$owner || ($owner != $vmid);
 
         $volids->{$volid} = 1;
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH qemu-server v8 3/4] destroy_vm: use log_warn for vdisk_free errors for consistency

[Michael Köppl]

To make warnings visually consistent with the handling of other storage
errors in destroy_vm(), replace the use of warn with log_warn.

Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
 src/PVE/QemuServer.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 91b0d1e0..27b4093b 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -1921,7 +1921,7 @@ sub destroy_vm {
 
         $volids->{$volid} = 1;
         eval { PVE::Storage::vdisk_free($storecfg, $volid) };
-        warn "Could not remove disk '$volid', check manually: $@" if $@;
+        log_warn("Could not remove disk '$volid', check manually: $@") if $@;
     };
 
     # only remove disks owned by this VM (referenced in the config)
@@ -1947,7 +1947,7 @@ sub destroy_vm {
             sub {
                 my ($volid, $sid, $volname, $d) = @_;
                 eval { PVE::Storage::vdisk_free($storecfg, $volid) };
-                warn $@ if $@;
+                log_warn($@) if $@;
             },
         );
     }
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH qemu-server v8 4/4] display warnings for storage errors or if storage no longer exists

[Michael Köppl]

Instead of continuing without informing the user, a warning will now be
displayed if the owner of a volume could not be determined due to a
storage error. In addition, an explicit check for the existence of the
underlying storage is added before the ownership check. If the storage
no longer exists, a warning will be displayed, consistent with the
handling of this scenario in other functions.

Signed-off-by: Michael Köppl <m.koeppl@proxmox.com>
---
 src/PVE/QemuServer.pm | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 27b4093b..71c5ce00 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -1599,6 +1599,7 @@ sub vm_is_volid_owner {
     if ($volid !~ m|^/|) {
         my ($path, $owner);
         eval { ($path, $owner) = PVE::Storage::path($storecfg, $volid); };
+        log_warn("ownership of volume '$volid' could not be determined: $@") if $@;
         if ($owner && ($owner == $vmid)) {
             return 1;
         }
@@ -1616,8 +1617,13 @@ sub vmconfig_register_unused_drive {
         delete $conf->{'special-sections'}->{cloudinit};
     } elsif (!drive_is_cdrom($drive)) {
         my $volid = $drive->{file};
-        if (vm_is_volid_owner($storecfg, $vmid, $volid)) {
-            PVE::QemuConfig->add_unused_volume($conf, $volid, $vmid);
+        my ($storeid, undef) = PVE::Storage::parse_volume_id($volid);
+        if (PVE::Storage::storage_config($storecfg, $storeid, 1)) {
+            if (vm_is_volid_owner($storecfg, $vmid, $volid)) {
+                PVE::QemuConfig->add_unused_volume($conf, $volid, $vmid);
+            }
+        } else {
+            log_warn("storage '$storeid' no longer exists, volume '$volid' will be removed");
         }
     }
 }
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH container/qemu-server/storage v8 00/10] fix #3711 and adapt drive detach/remove behavior

[Lukas Wagner]

On  2025-06-26 18:06, Michael Köppl wrote:
> This series aims to fix #3711 [0] and streamline the detach/remove
> behavior around volumes that are either mounted into a container or
> attached to a VM as a hard disk. It also contains adds warnings in case
> a volume's underlying storage does not exist anymore. It is a
> continuation of a series from 2022 [1], but makes the following changes:
> 

[...]

Gave these changes a quick test on the latest master.

I tested:
  - Remove {container,vm} after the the storage has been removed
     -> does not fail any more, but shows a warning
  - Remove a template if a linked clone exists
     -> now fails with an error


Something that I have noticed is that when you have a second disk/mountpoint on
some storage, detach the disk/mountpoint, remove the storage and then
try to remove the unused disk, there is a difference in behavior between
containers and VMs. For VMs, the ununused disk can be removed, while for containers
the removal fails with:

  unused0: unable to apply pending change unused0 : storage 'test' does not exist 


Also did a brief review of the code. I'm not really familiar with these parts
of our stack, but from my point of view these changes look fine.


Tested-by: Lukas Wagner <l.wagner@proxmox.com>
Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>


-- 
- Lukas



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Re: [pve-devel] [PATCH container/qemu-server/storage v8 00/10] fix #3711 and adapt drive detach/remove behavior

[Michael Köppl]

On 7/8/25 13:09, Lukas Wagner wrote:
> Something that I have noticed is that when you have a second disk/mountpoint on
> some storage, detach the disk/mountpoint, remove the storage and then
> try to remove the unused disk, there is a difference in behavior between
> containers and VMs. For VMs, the ununused disk can be removed, while for containers
> the removal fails with:
> 
>   unused0: unable to apply pending change unused0 : storage 'test' does not exist 
> 

Sorry for the late reply and thanks for having a look at this! Good
catch. Considering some of the other patches in this series are aimed at
streamlining the behavior of VMs and CTs, I'll add a check similar to
the one done for VMs in a v9.

> 
> Also did a brief review of the code. I'm not really familiar with these parts
> of our stack, but from my point of view these changes look fine.
> 
> 
> Tested-by: Lukas Wagner <l.wagner@proxmox.com>
> Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
> 
> 



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel