[pve-devel] [PATCH qemu-server] fix #2114: set correct link status on hotplug

[Dominik Csapak]

On 2/27/19 3:53 PM, Thomas Lamprecht wrote:
> On 2/27/19 2:40 PM, Dominik Csapak wrote:
>> we also need to set the link status if the whole device changed,
>> otherwise a change of macaddress allows a network connection even
>> if link_down is set to 1
>>
>> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
>> ---
>>   PVE/QemuServer.pm | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
>> index 6e56eda..3f190a3 100644
>> --- a/PVE/QemuServer.pm
>> +++ b/PVE/QemuServer.pm
>> @@ -4995,6 +4995,7 @@ sub vmconfig_update_net {
>>   
>>       if ($hotplug) {
>>   	vm_deviceplug($storecfg, $conf, $vmid, $opt, $newnet, $arch, $machine_type);
>> +	qemu_set_link_status($vmid, $opt, !$newnet->{link_down});
> 
> vm_deviceplug returns undef if it does not succeeds, would it make sense
> to only do this if it returns truthy?
> 
> if (vm_deviceplug($storecfg, $conf, $vmid, $opt, $newnet, $arch, $machine_type)) {
>      qemu_set_link_status($vmid, $opt, !$newnet->{link_down});
> }

in that case it would also make sense to have better error recovery?
or is silently failing ok here?

> 
> or move the qemu_set_link_status altogether in vm_deviceplug if branch for 'net'
> devices?

that is ok for me, i just wanted to have the places where we set the 
link together, so that one can see it is done in all necessary cases

> 
>>       } else {
>>   	die "skip\n";
>>       }
>>
>