[pve-devel] applied: [PATCH qemu-server 1/7] Move config removal after disk removal

[Thomas Lamprecht]

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 <d.jaeger at proxmox.com>
> ---
>  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 {
>