[pve-devel] [PATCH container/qemu-server/storage v7 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 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 | 9 +++++++++
 1 file changed, 9 insertions(+)


pve-container.git:

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.git:

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

 PVE/QemuServer.pm | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)


Summary over all repositories:
  4 files changed, 53 insertions(+), 21 deletions(-)

-- 
Generated by git-murpp 0.8.0