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

[Dominic Jäger]

On Fri, Oct 25, 2019 at 11:49:51AM +0200, Thomas Lamprecht wrote:
> 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.
> 
Makes sense, I'll pay more attention to this!

> > 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 {
> > 
> 
>