[pve-devel] [PATCH] don't try to hotplug disk if a disk already exist.

[Alexandre DERUMIER]

We need also to check if the vm is running and hotplug is enabled

(because without hotplug, we are allow to replace a disk with another, 
 the code manage currently the swap, putting the first disk as unused)

something like:
    elsif (PVE::QemuServer::check_running($vmid) && $conf->{hotplug}) { # hotplug new disks
        if($old_volid){
            die "you need to remove current disk before hotplug it" if $old_volid ne $volid;
        }else{
            die "error hotplug $opt" if !PVE::QemuServer::vm_deviceplug($storecfg, $conf, $vmid, $opt, $drive);
        }
    }



also, I found another bug, we update the conf with new_volid  before trying hotplug.
If hotplug fail, we had the wrong disk updated in config

I'll check the whole vmconfig_update_disk sub, to see to improve that



----- Mail original ----- 

De: "Dietmar Maurer" <dietmar at proxmox.com> 
À: "Alexandre DERUMIER" <aderumier at odiso.com> 
Cc: pve-devel at pve.proxmox.com 
Envoyé: Vendredi 29 Août 2014 11:26:00 
Objet: RE: [pve-devel] [PATCH] don't try to hotplug disk if a disk already exist. 

> >>what about this: 
> >> 
> >> } else { # hotplug new disks 
> >>+ die "some useful error mesage" if $old_volid; 
> >> die "error hotplug $opt" if !PVE::QemuServer::vm_deviceplug($storecfg, 
> $conf, $vmid, $opt, $drive); 
> >> } 
> >>} 
> 
> The problem is that we are in $vmconfig_update_disk(), 

die "some useful error mesage" if $old_volid && $old_volid ne $new_volid; 

> 
> so it'll die if we try to update any parameters (disk throttle,discard,backup). 
> 
> 
> ----- Mail original ----- 
> 
> De: "Dietmar Maurer" <dietmar at proxmox.com> 
> À: "Alexandre DERUMIER" <aderumier at odiso.com> 
> Cc: pve-devel at pve.proxmox.com 
> Envoyé: Vendredi 29 Août 2014 10:11:18 
> Objet: RE: [pve-devel] [PATCH] don't try to hotplug disk if a disk already exist. 
> 
> what about this: 
> 
> } else { # hotplug new disks 
> + die "some useful error mesage" if $old_volid; 
> die "error hotplug $opt" if !PVE::QemuServer::vm_deviceplug($storecfg, $conf, 
> $vmid, $opt, $drive); } } 
> 
> > -----Original Message----- 
> > From: Alexandre DERUMIER [mailto:aderumier at odiso.com] 
> > Sent: Freitag, 29. August 2014 09:25 
> > To: Dietmar Maurer 
> > Cc: pve-devel at pve.proxmox.com 
> > Subject: Re: [pve-devel] [PATCH] don't try to hotplug disk if a disk already 
> exist. 
> > 
> > >>This does not display any errors if $old_volid is set? 
> > >>I think we should raise an error to indicate that something went wrong? 
> > 
> > 
> > 
> > 
> > Maybe 
> > 
> > elsif (!$old_volid) { # hotplug new disks die "error hotplug $opt" if 
> > !PVE::QemuServer::vm_deviceplug($storecfg, 
> > $conf, $vmid, $opt, $drive); 
> > 
> > }elseif ($old_voldid && $old_voldid ne $new_volid { raise an error ? 
> > } 
> > 
> > 
> > ? 
> > 
> > ----- Mail original ----- 
> > 
> > De: "Dietmar Maurer" <dietmar at proxmox.com> 
> > À: "Alexandre Derumier" <aderumier at odiso.com>, pve- 
> > devel at pve.proxmox.com 
> > Envoyé: Vendredi 29 Août 2014 08:29:00 
> > Objet: RE: [pve-devel] [PATCH] don't try to hotplug disk if a disk already exist. 
> > 
> > > - } else { # hotplug new disks 
> > > - 
> > > + } elsif (!$old_volid) { # hotplug new disks 
> > > die "error hotplug $opt" if 
> > > !PVE::QemuServer::vm_deviceplug($storecfg, 
> > > $conf, $vmid, $opt, $drive); 
> > > } 
> > 
> > This does not display any errors if $old_volid is set? 
> > I think we should raise an error to indicate that something went wrong?