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

[Alexandre DERUMIER]

>> if($old_volid){
>>            die "you need to remove current disk before hotplug it" if $old_volid ne $volid;

also, I think $old_voldid is put as unused before in update_disk

 if (!PVE::QemuServer::drive_is_cdrom($old_drive) &&
                ($drive->{file} ne $old_drive->{file})) {  # delete old disks

                &$vmconfig_delete_option($rpcenv, $authuser, $conf, $storecfg, $vmid, $opt, $force);
                $conf = PVE::QemuServer::load_config($vmid); # update/reload
            }

as far I remember, it's working with hot-unplug too.

I'll do tests.



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

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

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? 
_______________________________________________ 
pve-devel mailing list 
pve-devel at pve.proxmox.com 
http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel