Proxmox/qemu-server
5
0
Fork 0
mirror of git://git.proxmox.com/git/qemu-server.git synced 2025-01-06 13:17:56 +03:00
Code Releases Activity

vm destroy: allow opt-out of purging unreferenced disks

Browse Source 
Since an old change released with a version bump on 2009-09-07, we
search all enabled storages for VMID maching volumes on VM removal
and purge those too.

This has multiple pitfalls and may be quite unexpected for some
users.

It can make problems when:
* on recovery a VM is created, before disks are reattached the admin
  notices some settings issues and chooses to just recreate the VM;
  but during destroying the dummy VM all related disks get destroyed
  unconditionally which may result in data loss. This actually
  happened and is the original reason for the decision to change
  this.

* a storage is shared between PVE instance (between a set of clusters
  and/or single nodes), while this is against our rules it may still
  come as a surprise if destroying a VM on node A may destroy
  unrelated and unreferenced disks on the unrelated node B without
  asking or allowing to avoid that.

As this the removal of matching but unreferenced disks can result in
permanent data loss (up to the last backup) and may be to subtle and
unforgiving, allow to opt-out of it.

In the long run we want to make this opt-in, but that is an API
change and so needs to wait for next major release. But, we can adapt
the GUI already to make it opt-in there, catching most of the cases.

side-note: CT do not have this behavior at all

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
This commit is contained in:
Thomas Lamprecht 2021-01-25 15:06:19 +01:00
parent 4405703d89
commit 7585466269
2 changed files with 24 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
PVE/API2/Qemu.pm
View File

@ -1517,6 +1517,12 @@ __PACKAGE__->register_method({
description => "Remove VMID from configurations, like backup & replication jobs and HA.",
optional => 1,
},
'destroy-unreferenced-disks' => {
type => 'boolean',
description => "If set, destroy all disks with the VMID from all enabled storages.",
optional => 1,
default => 1, # FIXME: replace to false in PVE 7.0, this is dangerous!
}
},
},
returns => {
@ -1566,7 +1572,15 @@ __PACKAGE__->register_method({
# repeat, config might have changed
my $ha_managed = $early_checks->();
PVE::QemuServer::destroy_vm($storecfg, $vmid, $skiplock, { lock => 'destroyed' });
# FIXME: drop fallback to true with 7.0, to dangerous for default
my $purge_unreferenced = $param->{'destroy-unreferenced-disks'} // 1;
PVE::QemuServer::destroy_vm(
$storecfg,
$vmid,
$skiplock, { lock => 'destroyed' },
$purge_unreferenced,
);
PVE::AccessControl::remove_vm_access($vmid);
PVE::Firewall::remove_vmfw_conf($vmid);

17
PVE/QemuServer.pm
View File

@ -2074,7 +2074,7 @@ sub check_type {
}
sub destroy_vm {
my ($storecfg, $vmid, $skiplock, $replacement_conf) = @_;
my ($storecfg, $vmid, $skiplock, $replacement_conf, $purge_unreferenced) = @_;
my $conf = PVE::QemuConfig->load_config($vmid);
@ -2110,13 +2110,14 @@ sub destroy_vm {
warn "Could not remove disk '$volid', check manually: $@" if $@;
});
# also remove unused disk
my $vmdisks = PVE::Storage::vdisk_list($storecfg, undef, $vmid);
PVE::Storage::foreach_volid($vmdisks, sub {
my ($volid, $sid, $volname, $d) = @_;
eval { PVE::Storage::vdisk_free($storecfg, $volid) };
warn $@ if $@;
});
if ($purge_unreferenced) { # also remove unreferenced disk
my $vmdisks = PVE::Storage::vdisk_list($storecfg, undef, $vmid);
PVE::Storage::foreach_volid($vmdisks, sub {
my ($volid, $sid, $volname, $d) = @_;
eval { PVE::Storage::vdisk_free($storecfg, $volid) };
warn $@ if $@;
});
}
if (defined $replacement_conf) {
PVE::QemuConfig->write_config($vmid, $replacement_conf);