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.jae...@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 {
> >
>
>
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
