[pve-devel] [PATCH container 1/1] Fix #2041: add spice webdav / folder sharing

[Dominik Csapak]

On 12/28/18 6:59 PM, Andreas Steinel wrote:
> Adding the device and serial port for the service spice-webdavd on Linux and
> Windows.
> 

hi, thanks for the patch, a few points though

only minor, but you have 'container' instead of 'qemu-server' in the 
subject prefix

> Signed-off-by: Andreas Steinel <A.Steinel at gmail.com>
> ---
>   PVE/QemuServer.pm     | 5 +++++
>   PVE/QemuServer/PCI.pm | 1 +
>   2 files changed, 6 insertions(+)
> 
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 1ccdccf..225f0c0 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -3767,6 +3767,11 @@ sub config_to_command {
>   	push @$devices, '-device', "virtio-serial,id=spice$pciaddr";
>   	push @$devices, '-chardev', "spicevmc,id=vdagent,name=vdagent";
>   	push @$devices, '-device', "virtserialport,chardev=vdagent,name=com.redhat.spice.0";
> +
> +	$pciaddr = print_pci_addr("spicewebdav", $bridges, $arch, $machine_type);
> +	push @$devices, '-device', "virtio-serial,id=webdav$pciaddr";
> +	push @$devices, '-chardev', "spiceport,id=webdav,name=org.spice-space.webdav.0";
> +	push @$devices, '-device', "virtserialport,chardev=webdav,name=org.spice-space.webdav.0";

without testing, this for sure breaks live migration (since there is now 
new hardware)

mainly there are 3 possibilities to include this by default with working 
live migration:

1. use our 'qemu version is greater than current' code, it then gets 
added with the next qemu update we will ship, and for migrated
vms this will only get included after a stop/start of the vm

2. add a new config option/property somewhere and add it by default in 
the wizard in the gui, this makes it only the default by vms created there

3. leave the code as is, but delay it until our next major version and
have it as a 'breaking change' (although i guess most users will not be 
happy with this)

alternatively, simple make it a config options somewhere, and expose it 
into the gui and let the user do it

>       }
>   
>       # enable balloon by default, unless explicitly disabled
> diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
> index f22f5ad..45c1c90 100644
> --- a/PVE/QemuServer/PCI.pm
> +++ b/PVE/QemuServer/PCI.pm
> @@ -68,6 +68,7 @@ my $devices = {
>       'net30' => { bus => 1, addr => 25 },
>       'net31' => { bus => 1, addr => 26 },
>       'xhci' => { bus => 1, addr => 27 },
> +    'spicewebdav' => { bus => 1, addr => 28 },
>       'virtio6' => { bus => 2, addr => 1 },
>       'virtio7' => { bus => 2, addr => 2 },
>       'virtio8' => { bus => 2, addr => 3 },
>