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
