On 10/25/19 11:24 AM, Dominic Jäger wrote:
> As mentioned on the mailing list [0] disks owned by the VM and unused
> disks should be removed before the config file is removed.
>
> [0] https://pve.proxmox.com/pipermail/pve-devel/2019-October/039593.html
>
> Signed-off-by: Dominic Jäger <[email protected]>
> ---
> PVE/QemuServer.pm | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
Applied, thanks! But modified subject line to:
"destroy vm: remove VM config *after* unused disk removal"
rationale:
* give some short idea what we touch (vm destroy)
* don't suggest that we removed it before touching any disk, but
specify that it was done before "unused" "only"
* "move config removal" is not 100% obvious to me, is it remove a
"move config" or move a "remove config" (I mean, that's probably
just my confusion but IMO it's easier to parse if you say what
changes in the order of the actions the code does, not how code
hunks are moved around)
Picky, I know, but commit message can help to understand the code,
not only when reviewing, but also when looking at it in the future,
tremendously.
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index d24de70..328a0d1 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -2592,12 +2592,6 @@ sub destroy_vm {
>
> });
>
> - if ($keep_empty_config) {
> - PVE::QemuConfig->write_config($vmid, { memory => 128 });
> - } else {
> - PVE::QemuConfig->destroy_config($vmid);
> - }
> -
> # also remove unused disk
> eval {
> my $dl = PVE::Storage::vdisk_list($storecfg, undef, $vmid);
> @@ -2612,6 +2606,12 @@ sub destroy_vm {
>
> };
> warn $@ if $@;
> +
> + if ($keep_empty_config) {
> + PVE::QemuConfig->write_config($vmid, { memory => 128 });
> + } else {
> + PVE::QemuConfig->destroy_config($vmid);
> + }
> }
>
> sub parse_vm_config {
>
_______________________________________________
pve-devel mailing list
[email protected]
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
