[pve-devel] [PATCH 1/6] vm_deviceplug|unplug : implement pending change

[Alexandre DERUMIER]

>>Why do you want to modify configuration here?
I think it was a lot easier with my previous implementation, but shouldnt require anymore.


>>if (vm_deviceplug(...)) { 
>>$conf->{$deviceid} = $optvalue; 
>>delete $conf->{pending}->{$deviceid}; 
>>PVE::QemuServer::update_config_nolock($vmid, $conf, 1); 
>>} 

>>(we can also make an extra sub for that) 


So, Yes, we can do like this.

I'll make a patch




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

De: "Dietmar Maurer" <dietmar at proxmox.com> 
À: "Alexandre Derumier" <aderumier at odiso.com>, pve-devel at pve.proxmox.com 
Envoyé: Mardi 18 Novembre 2014 11:19:21 
Objet: RE: [pve-devel] [PATCH 1/6] vm_deviceplug|unplug : implement pending change 


> Signed-off-by: Alexandre Derumier <aderumier at odiso.com> 
> --- 
> PVE/QemuServer.pm | 127 ++++++++++++++++++++++++++++++++++++--------- 
> -------- 
> 1 file changed, 86 insertions(+), 41 deletions(-) 
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm index e43a228..141a21c 
> 100644 
> --- a/PVE/QemuServer.pm 
> +++ b/PVE/QemuServer.pm 
> @@ -2994,63 +2994,81 @@ sub vm_devices_list { } 
> 
> sub vm_deviceplug { 
> - my ($storecfg, $conf, $vmid, $deviceid, $device) = @_; 
> + my ($storecfg, $conf, $vmid, $deviceid, $device, $optvalue) = @_; 
> 
> - return 1 if !check_running($vmid); 
> + if (!check_running($vmid)){ 
> + if($conf->{pending}->{$deviceid}){ 
> + $conf->{$deviceid} = $optvalue; 
> + delete $conf->{pending}->{$deviceid}; 
> + PVE::QemuServer::update_config_nolock($vmid, $conf, 1); 
> + } 
> + } 

Why do you want to modify configuration here? I am a bit afraid of side effect, because 
we call this function from several places? Maybe better to make the explicit, like: 

if (vm_deviceplug(...)) { 
$conf->{$deviceid} = $optvalue; 
delete $conf->{pending}->{$deviceid}; 
PVE::QemuServer::update_config_nolock($vmid, $conf, 1); 
} 

(we can also make an extra sub for that)