>> 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" <aderum...@odiso.com> À: "Dietmar Maurer" <diet...@proxmox.com> Cc: pve-devel@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" <diet...@proxmox.com> À: "Alexandre DERUMIER" <aderum...@odiso.com> Cc: pve-devel@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" <diet...@proxmox.com> > À: "Alexandre DERUMIER" <aderum...@odiso.com> > Cc: pve-devel@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:aderum...@odiso.com] > > Sent: Freitag, 29. August 2014 09:25 > > To: Dietmar Maurer > > Cc: pve-devel@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" <diet...@proxmox.com> > > À: "Alexandre Derumier" <aderum...@odiso.com>, pve- > > de...@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@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com http://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel