[pve-devel] [PATCH qemu-server] add ivshmem device to config

[Thomas Lamprecht]

On 1/24/19 3:52 PM, Dominik Csapak wrote:
> with such a shared memory device, a vm can share data with other
> vms or with the host via memory
>
> one of the use cases is looking-glass[1] with pci-passthrough, which copies
> the guest fb to the host and you get a high-speed, low-latency
> display client for the vm

did not really test the code, but a comment from pure review inline:

>
> 1: https://looking-glass.hostfission.com/
>
> Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
> ---
>  PVE/QemuServer.pm     | 43 +++++++++++++++++++++++++++++++++++++++++++
>  PVE/QemuServer/PCI.pm |  2 ++
>  2 files changed, 45 insertions(+)
>
> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
> index 1ccdccf..957f3a2 100644
> --- a/PVE/QemuServer.pm
> +++ b/PVE/QemuServer.pm
> @@ -248,6 +248,21 @@ my $vga_fmt = {
>      },
>  };
>  
> +my $ivshmem_fmt = {
> +    size => {
> +	type => 'integer',
> +	minimum => 1,
> +	description => "The size of the file in MB.",
> +    },
> +    name => {
> +	type => 'string',
> +	pattern => '[a-zA-Z0-9\-]+',
> +	optional => 1,
> +	format_description => 'string',
> +	description => "The name of the file. Will be prefixed with 'pve-shm-'. Default is the VMID.",
> +    },
> +};
> +
>  my $confdesc = {
>      onboot => {
>  	optional => 1,
> @@ -607,6 +622,12 @@ EODESCR
>  	default => "1 (autogenerated)",
>  	optional => 1,
>      },
> +    ivshmem => {
> +	type => 'string',
> +	format => $ivshmem_fmt,
> +	description => "Inter-VM shared memory. Useful for direct communication between VMs, or to the host.",
> +	optional => 1,
> +    }
>  };
>  
>  my $confdesc_cloudinit = {
> @@ -3876,6 +3897,24 @@ sub config_to_command {
>           push @$devices, '-device', $netdevicefull;
>      }
>  
> +    if ($conf->{ivshmem}) {
> +	my $ivshmem = PVE::JSONSchema::parse_property_string($ivshmem_fmt, $conf->{ivshmem});
> +	my $bus;
> +	if ($q35) {
> +	    $bus = print_pcie_addr("ivshmem");
> +	} else {
> +	    $bus = print_pci_addr("ivshmem", $bridges, $arch, $machine_type);
> +	}
> +	my $path = '/dev/shm/pve-shm-';
> +	if ($ivshmem->{name}) {
> +	    $path .= $ivshmem->{name};
> +	} else {
> +	    $path .= $vmid;
> +	}
> +	push @$devices, '-device', "ivshmem-plain,memdev=ivshmem$bus,";
> +	push @$devices, '-object', "memory-backend-file,id=ivshmem,share=on,mem-path=$path,size=$ivshmem->{size}M";
> +    }
> +
>      if (!$q35) {
>  	# add pci bridges
>          if (qemu_machine_feature_enabled ($machine_type, $kvmver, 2, 3)) {
> @@ -5419,6 +5458,10 @@ sub vm_stop_cleanup {
>  	    unlink "/var/run/qemu-server/${vmid}.$ext";
>  	}
>  
> +	# do not delete the file if a custom name was given,
> +	# there may be multiple vms accessing it
> +	unlink "/dev/shm/pve-shm-$vmid";

I mean, this heuristic could be wrong if someone adds it without a name,
then wants to share it and sets the $vmid based name in the second one.

Can we do this another, more error proof way? E.g., check if anybody has
an open FD on this?

> +
>  	foreach my $key (keys %$conf) {
>  	    next if $key !~ m/^hostpci(\d+)$/;
>  	    my $hostpciindex = $1;
> diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
> index f22f5ad..d847146 100644
> --- a/PVE/QemuServer/PCI.pm
> +++ b/PVE/QemuServer/PCI.pm
> @@ -78,6 +78,7 @@ my $devices = {
>      'virtio13' => { bus => 2, addr => 8 },
>      'virtio14' => { bus => 2, addr => 9 },
>      'virtio15' => { bus => 2, addr => 10 },
> +    'ivshmem' => { bus => 2, addr => 11 },
>      'virtioscsi0' => { bus => 3, addr => 1 },
>      'virtioscsi1' => { bus => 3, addr => 2 },
>      'virtioscsi2' => { bus => 3, addr => 3 },
> @@ -150,6 +151,7 @@ sub print_pcie_addr {
>  	hostpci1bus0 => { bus => "pcie.0", addr => 17 },
>  	hostpci2bus0 => { bus => "pcie.0", addr => 18 },
>  	hostpci3bus0 => { bus => "pcie.0", addr => 19 },
> +	ivshmem => { bus => 'pcie.0', addr => 20 },
>      };
>  
>      if (defined($devices->{$id}->{bus}) && defined($devices->{$id}->{addr})) {
>