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

[Thomas Lamprecht]

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"

> 
> [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+$/;
>