[pve-devel] [PATCH v3 qemu-server 2/3] Fix local resource check of Spice USB devices

[Aaron Lauterer]

On 9/10/19 11:03 AM, Thomas Lamprecht wrote:
> On 06.09.19 15:26, Aaron Lauterer wrote:
>> The check relied on the fact that in a regular use case a usb device of
>> type spice would not have any other settings like `usb3=1`. An exact
>> match worked fine for this.
>>
>> This patch changes the behaviour and makes it possible to migrate VMs
>> with Spice USB devices that have additional usb options set.
>>
>> Signed-off-by: Aaron Lauterer <a.lauterer at proxmox.com>
>> ---
>>
>> The proposed[0] /^spice(,)?$/ regex does not match the `usb3=1` part and
>> fails. I opted for the simpler regex that checks if `spice` is that the
>> beginning.
> 
> Then still the same applies as in my previous review:
> Still not really future proof.. The /spice(,)?/ was just a not-much-thought-out
> suggestion to highlight the direction one could go, so instead of going back may
> try the now full-thought-out-and-tested:
> 
> /^spice(?![^,])/
> 
> Iow, matches a string starting with spice which, if it isn't finished, may not
> be followed by anything else as a comma, check `perldoc perlre` for
> "negative lookahead"
> 

Thanks for the explanation, now I got it what you were trying to do.

>>
>> [0]: https://pve.proxmox.com/pipermail/pve-devel/2019-September/038749.html
>>
>>   PVE/QemuServer.pm | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
>> index 0489b27..5334cad 100644
>> --- a/PVE/QemuServer.pm
>> +++ b/PVE/QemuServer.pm
>> @@ -2925,7 +2925,7 @@ sub check_local_resources {
>>       push @loc_res, "ivshmem" if $conf->{ivshmem};
>>   
>>       foreach my $k (keys %$conf) {
>> -	next if $k =~ m/^usb/ && ($conf->{$k} eq 'spice');
>> +	next if $k =~ m/^usb/ && ($conf->{$k} =~ m/^spice/);
>>   	# sockets are safe: they will recreated be on the target side post-migrate
>>   	next if $k =~ m/^serial/ && ($conf->{$k} eq 'socket');
>>   	push @loc_res, $k if $k =~ m/^(usb|hostpci|serial|parallel)\d+$/;
>>
>