[Qemu-trivial] [PATCH RESEND] ivshmem: allow the sharing of hugepages

[Qemu-trivial] [PATCH RESEND] ivshmem: allow the sharing of hugepages

[Damien Millescamps]

This patch permits to share memory areas that do not specifically belong to
/dev/shm. In such case, the file must be already present when launching qemu.

A use case for this patch is sharing huge pages available through a
hugetlbfs mountpoint.

Signed-off-by: Damien Millescamps <damien.millescamps@6wind.com>
---
 docs/specs/ivshmem_device_spec.txt |    5 +++--
 hw/misc/ivshmem.c                  |   10 +++++++---
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/docs/specs/ivshmem_device_spec.txt b/docs/specs/ivshmem_device_spec.txt
index 667a862..3137e60 100644
--- a/docs/specs/ivshmem_device_spec.txt
+++ b/docs/specs/ivshmem_device_spec.txt
@@ -4,8 +4,9 @@ Device Specification for Inter-VM shared memory device
 
 The Inter-VM shared memory device is designed to share a region of memory to
 userspace in multiple virtual guests.  The memory region does not belong to any
-guest, but is a POSIX memory object on the host.  Optionally, the device may
-support sending interrupts to other guests sharing the same memory region.
+guest, but is a either a POSIX memory object or a mmap'd file (such as a
+hugepage) on the host.  Optionally, the device may support sending interrupts
+to other guests sharing the same memory region.
 
 
 The Inter-VM PCI device
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 2838866..9178ccc 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -762,9 +762,13 @@ static int pci_ivshmem_init(PCIDevice *dev)
 
         } else if ((fd = shm_open(s->shmobj, O_CREAT|O_RDWR,
                         S_IRWXU|S_IRWXG|S_IRWXO)) < 0) {
-            fprintf(stderr, "ivshmem: could not open shared file\n");
-            exit(-1);
-
+            /* Try with open() in case the file is not in /dev/shm/
+             * This is usefull for sharing hugepages for example */
+            fd = open(s->shmobj, O_RDWR);
+            if (fd < 0) {
+                fprintf(stderr, "ivshmem: could not open shared file\n");
+                exit(-1);
+            }
         }
 
         if (check_shm_size(s, fd) == -1) {
-- 
1.7.2.5

Re: [Qemu-trivial] [Qemu-devel] [PATCH RESEND] ivshmem: allow the sharing of hugepages

[Daniel P. Berrange]

On Mon, Sep 16, 2013 at 02:56:15PM +0200, Damien Millescamps wrote:
> This patch permits to share memory areas that do not specifically belong to
> /dev/shm. In such case, the file must be already present when launching qemu.
> 
> A use case for this patch is sharing huge pages available through a
> hugetlbfs mountpoint.
> 
> Signed-off-by: Damien Millescamps <damien.millescamps@6wind.com>
> ---
>  docs/specs/ivshmem_device_spec.txt |    5 +++--
>  hw/misc/ivshmem.c                  |   10 +++++++---
>  2 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/docs/specs/ivshmem_device_spec.txt b/docs/specs/ivshmem_device_spec.txt
> index 667a862..3137e60 100644
> --- a/docs/specs/ivshmem_device_spec.txt
> +++ b/docs/specs/ivshmem_device_spec.txt
> @@ -4,8 +4,9 @@ Device Specification for Inter-VM shared memory device
>  
>  The Inter-VM shared memory device is designed to share a region of memory to
>  userspace in multiple virtual guests.  The memory region does not belong to any
> -guest, but is a POSIX memory object on the host.  Optionally, the device may
> -support sending interrupts to other guests sharing the same memory region.
> +guest, but is a either a POSIX memory object or a mmap'd file (such as a
> +hugepage) on the host.  Optionally, the device may support sending interrupts
> +to other guests sharing the same memory region.
>  
>  
>  The Inter-VM PCI device
> diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> index 2838866..9178ccc 100644
> --- a/hw/misc/ivshmem.c
> +++ b/hw/misc/ivshmem.c
> @@ -762,9 +762,13 @@ static int pci_ivshmem_init(PCIDevice *dev)
>  
>          } else if ((fd = shm_open(s->shmobj, O_CREAT|O_RDWR,
>                          S_IRWXU|S_IRWXG|S_IRWXO)) < 0) {
> -            fprintf(stderr, "ivshmem: could not open shared file\n");
> -            exit(-1);
> -
> +            /* Try with open() in case the file is not in /dev/shm/
> +             * This is usefull for sharing hugepages for example */
> +            fd = open(s->shmobj, O_RDWR);
> +            if (fd < 0) {
> +                fprintf(stderr, "ivshmem: could not open shared file\n");
> +                exit(-1);
> +            }

IME this kind of auto-magical fallback behaviour is a bad idea. If we
want to support non-SHM files for the ivshmem device, then it should
be done with an explicit command line property. eg where we currently
have a 'size' and 'shm' property on the cli:

   -device ivshmem,size=24124324,shm=nameofshmobj

it could allow an alternative 'file' property to point to a pre-created
filename

   -device ivshmem,file=/some/file/path


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-trivial] [Qemu-devel] [PATCH RESEND] ivshmem: allow the sharing of hugepages

[Damien Millescamps]

On 09/16/2013 03:07 PM, Daniel P. Berrange wrote:
> IME this kind of auto-magical fallback behaviour is a bad idea. If we
> want to support non-SHM files for the ivshmem device, then it should
> be done with an explicit command line property. eg where we currently
> have a 'size' and 'shm' property on the cli:
>
>    -device ivshmem,size=24124324,shm=nameofshmobj
>
> it could allow an alternative 'file' property to point to a pre-created
> filename
>
>    -device ivshmem,file=/some/file/path
This patch was based on the fact that in the glibc, shm_open() is
nothing more than an open() with /dev/shm/ prepended to the shared
memory object name. Hence, only a small modification was needed to
change this behavior.

Indeed, having different parameters to specify between a file and a
POSIX shared memory object should be clearer for the user experience.

I have reworked the patch and will send it asap.

Thanks,
-- 
Damien