[Qemu-devel] [PATCH] hostmem-memfd: honour share=on/off property

[Qemu-devel] [PATCH] hostmem-memfd: honour share=on/off property

[Marc-André Lureau]

The share=on/off property is used to modified mmap() MAP_SHARED
setting. Make it on by default for convenience and compatibility
reasons.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 backends/hostmem-memfd.c | 4 +++-
 qemu-options.hx          | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
index 1e20fe0ba8..abd52364db 100644
--- a/backends/hostmem-memfd.c
+++ b/backends/hostmem-memfd.c
@@ -59,7 +59,8 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
 
     name = object_get_canonical_path(OBJECT(backend));
     memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend),
-                                   name, backend->size, true, fd, errp);
+                                   name, backend->size,
+                                   backend->share, fd, errp);
     g_free(name);
 }
 
@@ -131,6 +132,7 @@ memfd_backend_instance_init(Object *obj)
 
     /* default to sealed file */
     m->seal = true;
+    MEMORY_BACKEND(m)->share = true;
 }
 
 static void
diff --git a/qemu-options.hx b/qemu-options.hx
index f14e423b7c..25c02d72b8 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4029,7 +4029,7 @@ Memory backend objects offer more control than the @option{-m} option that is
 traditionally used to define guest RAM. Please refer to
 @option{memory-backend-file} for a description of the options.
 
-@item -object memory-backend-memfd,id=@var{id},merge=@var{on|off},dump=@var{on|off},prealloc=@var{on|off},size=@var{size},host-nodes=@var{host-nodes},policy=@var{default|preferred|bind|interleave},seal=@var{on|off},hugetlb=@var{on|off},hugetlbsize=@var{size}
+@item -object memory-backend-memfd,id=@var{id},merge=@var{on|off},dump=@var{on|off},share=@var{on|off},prealloc=@var{on|off},size=@var{size},host-nodes=@var{host-nodes},policy=@var{default|preferred|bind|interleave},seal=@var{on|off},hugetlb=@var{on|off},hugetlbsize=@var{size}
 
 Creates an anonymous memory file backend object, which allows QEMU to
 share the memory with an external process (e.g. when using
@@ -4051,6 +4051,8 @@ with the @option{seal} option (requires at least Linux 4.16).
 Please refer to @option{memory-backend-file} for a description of the
 other options.
 
+The @option{share} boolean option is @var{on} by default with memfd.
+
 @item -object rng-random,id=@var{id},filename=@var{/dev/random}
 
 Creates a random number generator backend which obtains entropy from
-- 
2.19.0.rc0.48.gb9dfa238d5

Re: [Qemu-devel] [PATCH] hostmem-memfd: honour share=on/off property

[Igor Mammedov]

On Tue, 28 Aug 2018 17:38:40 +0200
Marc-André Lureau <marcandre.lureau@redhat.com> wrote:

> The share=on/off property is used to modified mmap() MAP_SHARED
> setting. Make it on by default for convenience and compatibility
> reasons.
it would be nice to state reasons here.

also flipping default to on is behavioral change when
used with bind policy which could make backend initialization fail
so CLI that used to work (if there are any users of this without shared=on)
won't work anymore.

> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  backends/hostmem-memfd.c | 4 +++-
>  qemu-options.hx          | 4 +++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
> index 1e20fe0ba8..abd52364db 100644
> --- a/backends/hostmem-memfd.c
> +++ b/backends/hostmem-memfd.c
> @@ -59,7 +59,8 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>  
>      name = object_get_canonical_path(OBJECT(backend));
>      memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend),
> -                                   name, backend->size, true, fd, errp);
> +                                   name, backend->size,
> +                                   backend->share, fd, errp);
>      g_free(name);
>  }
>  
> @@ -131,6 +132,7 @@ memfd_backend_instance_init(Object *obj)
>  
>      /* default to sealed file */
>      m->seal = true;
> +    MEMORY_BACKEND(m)->share = true;
>  }
>  
>  static void
> diff --git a/qemu-options.hx b/qemu-options.hx
> index f14e423b7c..25c02d72b8 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4029,7 +4029,7 @@ Memory backend objects offer more control than the @option{-m} option that is
>  traditionally used to define guest RAM. Please refer to
>  @option{memory-backend-file} for a description of the options.
>  
> -@item -object memory-backend-memfd,id=@var{id},merge=@var{on|off},dump=@var{on|off},prealloc=@var{on|off},size=@var{size},host-nodes=@var{host-nodes},policy=@var{default|preferred|bind|interleave},seal=@var{on|off},hugetlb=@var{on|off},hugetlbsize=@var{size}
> +@item -object memory-backend-memfd,id=@var{id},merge=@var{on|off},dump=@var{on|off},share=@var{on|off},prealloc=@var{on|off},size=@var{size},host-nodes=@var{host-nodes},policy=@var{default|preferred|bind|interleave},seal=@var{on|off},hugetlb=@var{on|off},hugetlbsize=@var{size}
>  
>  Creates an anonymous memory file backend object, which allows QEMU to
>  share the memory with an external process (e.g. when using
> @@ -4051,6 +4051,8 @@ with the @option{seal} option (requires at least Linux 4.16).
>  Please refer to @option{memory-backend-file} for a description of the
>  other options.
>  
> +The @option{share} boolean option is @var{on} by default with memfd.
> +
>  @item -object rng-random,id=@var{id},filename=@var{/dev/random}
>  
>  Creates a random number generator backend which obtains entropy from

Re: [Qemu-devel] [PATCH] hostmem-memfd: honour share=on/off property

[Marc-André Lureau]

Hi

On Tue, Aug 28, 2018 at 6:49 PM Igor Mammedov <imammedo@redhat.com> wrote:
>
> On Tue, 28 Aug 2018 17:38:40 +0200
> Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
>
> > The share=on/off property is used to modified mmap() MAP_SHARED
> > setting. Make it on by default for convenience and compatibility
> > reasons.
> it would be nice to state reasons here.
>
> also flipping default to on is behavioral change when
> used with bind policy which could make backend initialization fail
> so CLI that used to work (if there are any users of this without shared=on)
> won't work anymore.

memfd backend is passing shared=true to memory_region_init_ram_from_fd().

The patch allows to tweak that (instead of ignoring the property),
while keeping the value to shared=true.

>
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  backends/hostmem-memfd.c | 4 +++-
> >  qemu-options.hx          | 4 +++-
> >  2 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
> > index 1e20fe0ba8..abd52364db 100644
> > --- a/backends/hostmem-memfd.c
> > +++ b/backends/hostmem-memfd.c
> > @@ -59,7 +59,8 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
> >
> >      name = object_get_canonical_path(OBJECT(backend));
> >      memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend),
> > -                                   name, backend->size, true, fd, errp);
> > +                                   name, backend->size,
> > +                                   backend->share, fd, errp);
> >      g_free(name);
> >  }
> >
> > @@ -131,6 +132,7 @@ memfd_backend_instance_init(Object *obj)
> >
> >      /* default to sealed file */
> >      m->seal = true;
> > +    MEMORY_BACKEND(m)->share = true;
> >  }
> >
> >  static void
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index f14e423b7c..25c02d72b8 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -4029,7 +4029,7 @@ Memory backend objects offer more control than the @option{-m} option that is
> >  traditionally used to define guest RAM. Please refer to
> >  @option{memory-backend-file} for a description of the options.
> >
> > -@item -object memory-backend-memfd,id=@var{id},merge=@var{on|off},dump=@var{on|off},prealloc=@var{on|off},size=@var{size},host-nodes=@var{host-nodes},policy=@var{default|preferred|bind|interleave},seal=@var{on|off},hugetlb=@var{on|off},hugetlbsize=@var{size}
> > +@item -object memory-backend-memfd,id=@var{id},merge=@var{on|off},dump=@var{on|off},share=@var{on|off},prealloc=@var{on|off},size=@var{size},host-nodes=@var{host-nodes},policy=@var{default|preferred|bind|interleave},seal=@var{on|off},hugetlb=@var{on|off},hugetlbsize=@var{size}
> >
> >  Creates an anonymous memory file backend object, which allows QEMU to
> >  share the memory with an external process (e.g. when using
> > @@ -4051,6 +4051,8 @@ with the @option{seal} option (requires at least Linux 4.16).
> >  Please refer to @option{memory-backend-file} for a description of the
> >  other options.
> >
> > +The @option{share} boolean option is @var{on} by default with memfd.
> > +
> >  @item -object rng-random,id=@var{id},filename=@var{/dev/random}
> >
> >  Creates a random number generator backend which obtains entropy from
>
>


-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH] hostmem-memfd: honour share=on/off property

[Marc-André Lureau]

On Tue, Aug 28, 2018 at 6:54 PM Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi
>
> On Tue, Aug 28, 2018 at 6:49 PM Igor Mammedov <imammedo@redhat.com> wrote:
> >
> > On Tue, 28 Aug 2018 17:38:40 +0200
> > Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
> >
> > > The share=on/off property is used to modified mmap() MAP_SHARED
> > > setting. Make it on by default for convenience and compatibility
> > > reasons.
> > it would be nice to state reasons here.
> >
> > also flipping default to on is behavioral change when
> > used with bind policy which could make backend initialization fail
> > so CLI that used to work (if there are any users of this without shared=on)
> > won't work anymore.
>
> memfd backend is passing shared=true to memory_region_init_ram_from_fd().
>
> The patch allows to tweak that (instead of ignoring the property),
> while keeping the value to shared=true.

keeping the default value to shared=trrue.

>
> >
> > >
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > ---
> > >  backends/hostmem-memfd.c | 4 +++-
> > >  qemu-options.hx          | 4 +++-
> > >  2 files changed, 6 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
> > > index 1e20fe0ba8..abd52364db 100644
> > > --- a/backends/hostmem-memfd.c
> > > +++ b/backends/hostmem-memfd.c
> > > @@ -59,7 +59,8 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
> > >
> > >      name = object_get_canonical_path(OBJECT(backend));
> > >      memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend),
> > > -                                   name, backend->size, true, fd, errp);
> > > +                                   name, backend->size,
> > > +                                   backend->share, fd, errp);
> > >      g_free(name);
> > >  }
> > >
> > > @@ -131,6 +132,7 @@ memfd_backend_instance_init(Object *obj)
> > >
> > >      /* default to sealed file */
> > >      m->seal = true;
> > > +    MEMORY_BACKEND(m)->share = true;
> > >  }
> > >
> > >  static void
> > > diff --git a/qemu-options.hx b/qemu-options.hx
> > > index f14e423b7c..25c02d72b8 100644
> > > --- a/qemu-options.hx
> > > +++ b/qemu-options.hx
> > > @@ -4029,7 +4029,7 @@ Memory backend objects offer more control than the @option{-m} option that is
> > >  traditionally used to define guest RAM. Please refer to
> > >  @option{memory-backend-file} for a description of the options.
> > >
> > > -@item -object memory-backend-memfd,id=@var{id},merge=@var{on|off},dump=@var{on|off},prealloc=@var{on|off},size=@var{size},host-nodes=@var{host-nodes},policy=@var{default|preferred|bind|interleave},seal=@var{on|off},hugetlb=@var{on|off},hugetlbsize=@var{size}
> > > +@item -object memory-backend-memfd,id=@var{id},merge=@var{on|off},dump=@var{on|off},share=@var{on|off},prealloc=@var{on|off},size=@var{size},host-nodes=@var{host-nodes},policy=@var{default|preferred|bind|interleave},seal=@var{on|off},hugetlb=@var{on|off},hugetlbsize=@var{size}
> > >
> > >  Creates an anonymous memory file backend object, which allows QEMU to
> > >  share the memory with an external process (e.g. when using
> > > @@ -4051,6 +4051,8 @@ with the @option{seal} option (requires at least Linux 4.16).
> > >  Please refer to @option{memory-backend-file} for a description of the
> > >  other options.
> > >
> > > +The @option{share} boolean option is @var{on} by default with memfd.
> > > +
> > >  @item -object rng-random,id=@var{id},filename=@var{/dev/random}
> > >
> > >  Creates a random number generator backend which obtains entropy from
> >
> >
>
>
> --
> Marc-André Lureau



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH] hostmem-memfd: honour share=on/off property

[Igor Mammedov]

On Tue, 28 Aug 2018 18:55:35 +0200
Marc-André Lureau <marcandre.lureau@gmail.com> wrote:

> On Tue, Aug 28, 2018 at 6:54 PM Marc-André Lureau
> <marcandre.lureau@gmail.com> wrote:
> >
> > Hi
> >
> > On Tue, Aug 28, 2018 at 6:49 PM Igor Mammedov <imammedo@redhat.com> wrote:  
> > >
> > > On Tue, 28 Aug 2018 17:38:40 +0200
> > > Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
> > >  
> > > > The share=on/off property is used to modified mmap() MAP_SHARED
> > > > setting. Make it on by default for convenience and compatibility
> > > > reasons.  
> > > it would be nice to state reasons here.
> > >
> > > also flipping default to on is behavioral change when
> > > used with bind policy which could make backend initialization fail
> > > so CLI that used to work (if there are any users of this without shared=on)
> > > won't work anymore.  
> >
> > memfd backend is passing shared=true to memory_region_init_ram_from_fd().
> >
> > The patch allows to tweak that (instead of ignoring the property),
> > while keeping the value to shared=true.  
> 
> keeping the default value to shared=trrue.
ok,
another question:
 does it make sense to allow "shared=false" at all?
 should we bail out if user asks for it?

> 
> >  
> > >  
> > > >
> > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > > ---
> > > >  backends/hostmem-memfd.c | 4 +++-
> > > >  qemu-options.hx          | 4 +++-
> > > >  2 files changed, 6 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
> > > > index 1e20fe0ba8..abd52364db 100644
> > > > --- a/backends/hostmem-memfd.c
> > > > +++ b/backends/hostmem-memfd.c
> > > > @@ -59,7 +59,8 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
> > > >
> > > >      name = object_get_canonical_path(OBJECT(backend));
> > > >      memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend),
> > > > -                                   name, backend->size, true, fd, errp);
> > > > +                                   name, backend->size,
> > > > +                                   backend->share, fd, errp);
> > > >      g_free(name);
> > > >  }
> > > >
> > > > @@ -131,6 +132,7 @@ memfd_backend_instance_init(Object *obj)
> > > >
> > > >      /* default to sealed file */
> > > >      m->seal = true;
> > > > +    MEMORY_BACKEND(m)->share = true;
> > > >  }
> > > >
> > > >  static void
> > > > diff --git a/qemu-options.hx b/qemu-options.hx
> > > > index f14e423b7c..25c02d72b8 100644
> > > > --- a/qemu-options.hx
> > > > +++ b/qemu-options.hx
> > > > @@ -4029,7 +4029,7 @@ Memory backend objects offer more control than the @option{-m} option that is
> > > >  traditionally used to define guest RAM. Please refer to
> > > >  @option{memory-backend-file} for a description of the options.
> > > >
> > > > -@item -object memory-backend-memfd,id=@var{id},merge=@var{on|off},dump=@var{on|off},prealloc=@var{on|off},size=@var{size},host-nodes=@var{host-nodes},policy=@var{default|preferred|bind|interleave},seal=@var{on|off},hugetlb=@var{on|off},hugetlbsize=@var{size}
> > > > +@item -object memory-backend-memfd,id=@var{id},merge=@var{on|off},dump=@var{on|off},share=@var{on|off},prealloc=@var{on|off},size=@var{size},host-nodes=@var{host-nodes},policy=@var{default|preferred|bind|interleave},seal=@var{on|off},hugetlb=@var{on|off},hugetlbsize=@var{size}
> > > >
> > > >  Creates an anonymous memory file backend object, which allows QEMU to
> > > >  share the memory with an external process (e.g. when using
> > > > @@ -4051,6 +4051,8 @@ with the @option{seal} option (requires at least Linux 4.16).
> > > >  Please refer to @option{memory-backend-file} for a description of the
> > > >  other options.
> > > >
> > > > +The @option{share} boolean option is @var{on} by default with memfd.
> > > > +
> > > >  @item -object rng-random,id=@var{id},filename=@var{/dev/random}
> > > >
> > > >  Creates a random number generator backend which obtains entropy from  
> > >
> > >  
> >
> >
> > --
> > Marc-André Lureau  
> 
> 
> 

Re: [Qemu-devel] [PATCH] hostmem-memfd: honour share=on/off property

[Marc-André Lureau]

Hi

On Wed, Aug 29, 2018 at 10:50 AM Igor Mammedov <imammedo@redhat.com> wrote:
>
> On Tue, 28 Aug 2018 18:55:35 +0200
> Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
>
> > On Tue, Aug 28, 2018 at 6:54 PM Marc-André Lureau
> > <marcandre.lureau@gmail.com> wrote:
> > >
> > > Hi
> > >
> > > On Tue, Aug 28, 2018 at 6:49 PM Igor Mammedov <imammedo@redhat.com> wrote:
> > > >
> > > > On Tue, 28 Aug 2018 17:38:40 +0200
> > > > Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
> > > >
> > > > > The share=on/off property is used to modified mmap() MAP_SHARED
> > > > > setting. Make it on by default for convenience and compatibility
> > > > > reasons.
> > > > it would be nice to state reasons here.
> > > >
> > > > also flipping default to on is behavioral change when
> > > > used with bind policy which could make backend initialization fail
> > > > so CLI that used to work (if there are any users of this without shared=on)
> > > > won't work anymore.
> > >
> > > memfd backend is passing shared=true to memory_region_init_ram_from_fd().
> > >
> > > The patch allows to tweak that (instead of ignoring the property),
> > > while keeping the value to shared=true.
> >
> > keeping the default value to shared=trrue.
> ok,
> another question:
>  does it make sense to allow "shared=false" at all?
>  should we bail out if user asks for it?

You may want to use memfd for convenience (no need to specify hugetlb
mem-path for ex), without the share memory aspects.

(but the patch was mostly for completeness)

>
> >
> > >
> > > >
> > > > >
> > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > > > ---
> > > > >  backends/hostmem-memfd.c | 4 +++-
> > > > >  qemu-options.hx          | 4 +++-
> > > > >  2 files changed, 6 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
> > > > > index 1e20fe0ba8..abd52364db 100644
> > > > > --- a/backends/hostmem-memfd.c
> > > > > +++ b/backends/hostmem-memfd.c
> > > > > @@ -59,7 +59,8 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
> > > > >
> > > > >      name = object_get_canonical_path(OBJECT(backend));
> > > > >      memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend),
> > > > > -                                   name, backend->size, true, fd, errp);
> > > > > +                                   name, backend->size,
> > > > > +                                   backend->share, fd, errp);
> > > > >      g_free(name);
> > > > >  }
> > > > >
> > > > > @@ -131,6 +132,7 @@ memfd_backend_instance_init(Object *obj)
> > > > >
> > > > >      /* default to sealed file */
> > > > >      m->seal = true;
> > > > > +    MEMORY_BACKEND(m)->share = true;
> > > > >  }
> > > > >
> > > > >  static void
> > > > > diff --git a/qemu-options.hx b/qemu-options.hx
> > > > > index f14e423b7c..25c02d72b8 100644
> > > > > --- a/qemu-options.hx
> > > > > +++ b/qemu-options.hx
> > > > > @@ -4029,7 +4029,7 @@ Memory backend objects offer more control than the @option{-m} option that is
> > > > >  traditionally used to define guest RAM. Please refer to
> > > > >  @option{memory-backend-file} for a description of the options.
> > > > >
> > > > > -@item -object memory-backend-memfd,id=@var{id},merge=@var{on|off},dump=@var{on|off},prealloc=@var{on|off},size=@var{size},host-nodes=@var{host-nodes},policy=@var{default|preferred|bind|interleave},seal=@var{on|off},hugetlb=@var{on|off},hugetlbsize=@var{size}
> > > > > +@item -object memory-backend-memfd,id=@var{id},merge=@var{on|off},dump=@var{on|off},share=@var{on|off},prealloc=@var{on|off},size=@var{size},host-nodes=@var{host-nodes},policy=@var{default|preferred|bind|interleave},seal=@var{on|off},hugetlb=@var{on|off},hugetlbsize=@var{size}
> > > > >
> > > > >  Creates an anonymous memory file backend object, which allows QEMU to
> > > > >  share the memory with an external process (e.g. when using
> > > > > @@ -4051,6 +4051,8 @@ with the @option{seal} option (requires at least Linux 4.16).
> > > > >  Please refer to @option{memory-backend-file} for a description of the
> > > > >  other options.
> > > > >
> > > > > +The @option{share} boolean option is @var{on} by default with memfd.
> > > > > +
> > > > >  @item -object rng-random,id=@var{id},filename=@var{/dev/random}
> > > > >
> > > > >  Creates a random number generator backend which obtains entropy from
> > > >
> > > >
> > >
> > >
> > > --
> > > Marc-André Lureau
> >
> >
> >
>


-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH] hostmem-memfd: honour share=on/off property

[Igor Mammedov]

On Tue, 28 Aug 2018 17:38:40 +0200
Marc-André Lureau <marcandre.lureau@redhat.com> wrote:

> The share=on/off property is used to modified mmap() MAP_SHARED
> setting. Make it on by default for convenience and compatibility
> reasons.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  backends/hostmem-memfd.c | 4 +++-
>  qemu-options.hx          | 4 +++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
> index 1e20fe0ba8..abd52364db 100644
> --- a/backends/hostmem-memfd.c
> +++ b/backends/hostmem-memfd.c
> @@ -59,7 +59,8 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>  
>      name = object_get_canonical_path(OBJECT(backend));
>      memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend),
> -                                   name, backend->size, true, fd, errp);
> +                                   name, backend->size,
> +                                   backend->share, fd, errp);
>      g_free(name);
>  }
>  
> @@ -131,6 +132,7 @@ memfd_backend_instance_init(Object *obj)
>  
>      /* default to sealed file */
>      m->seal = true;
> +    MEMORY_BACKEND(m)->share = true;
>  }
>  
>  static void
> diff --git a/qemu-options.hx b/qemu-options.hx
> index f14e423b7c..25c02d72b8 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4029,7 +4029,7 @@ Memory backend objects offer more control than the @option{-m} option that is
>  traditionally used to define guest RAM. Please refer to
>  @option{memory-backend-file} for a description of the options.
>  
> -@item -object memory-backend-memfd,id=@var{id},merge=@var{on|off},dump=@var{on|off},prealloc=@var{on|off},size=@var{size},host-nodes=@var{host-nodes},policy=@var{default|preferred|bind|interleave},seal=@var{on|off},hugetlb=@var{on|off},hugetlbsize=@var{size}
> +@item -object memory-backend-memfd,id=@var{id},merge=@var{on|off},dump=@var{on|off},share=@var{on|off},prealloc=@var{on|off},size=@var{size},host-nodes=@var{host-nodes},policy=@var{default|preferred|bind|interleave},seal=@var{on|off},hugetlb=@var{on|off},hugetlbsize=@var{size}
>  
>  Creates an anonymous memory file backend object, which allows QEMU to
>  share the memory with an external process (e.g. when using
> @@ -4051,6 +4051,8 @@ with the @option{seal} option (requires at least Linux 4.16).
>  Please refer to @option{memory-backend-file} for a description of the
>  other options.
>  
> +The @option{share} boolean option is @var{on} by default with memfd.
> +
>  @item -object rng-random,id=@var{id},filename=@var{/dev/random}
>  
>  Creates a random number generator backend which obtains entropy from

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

Re: [Qemu-devel] [PATCH] hostmem-memfd: honour share=on/off property

[Igor Mammedov]

On Wed, 29 Aug 2018 14:36:00 +0200
Marc-André Lureau <marcandre.lureau@gmail.com> wrote:

> Hi
> 
> On Wed, Aug 29, 2018 at 10:50 AM Igor Mammedov <imammedo@redhat.com> wrote:
> >
> > On Tue, 28 Aug 2018 18:55:35 +0200
> > Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
> >  
> > > On Tue, Aug 28, 2018 at 6:54 PM Marc-André Lureau
> > > <marcandre.lureau@gmail.com> wrote:  
> > > >
> > > > Hi
> > > >
> > > > On Tue, Aug 28, 2018 at 6:49 PM Igor Mammedov <imammedo@redhat.com> wrote:  
> > > > >
> > > > > On Tue, 28 Aug 2018 17:38:40 +0200
> > > > > Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
> > > > >  
> > > > > > The share=on/off property is used to modified mmap() MAP_SHARED
> > > > > > setting. Make it on by default for convenience and compatibility
> > > > > > reasons.  
> > > > > it would be nice to state reasons here.
> > > > >
> > > > > also flipping default to on is behavioral change when
> > > > > used with bind policy which could make backend initialization fail
> > > > > so CLI that used to work (if there are any users of this without shared=on)
> > > > > won't work anymore.  
> > > >
> > > > memfd backend is passing shared=true to memory_region_init_ram_from_fd().
> > > >
> > > > The patch allows to tweak that (instead of ignoring the property),
> > > > while keeping the value to shared=true.  
> > >
> > > keeping the default value to shared=trrue.  
> > ok,
> > another question:
> >  does it make sense to allow "shared=false" at all?
> >  should we bail out if user asks for it?  
> 
> You may want to use memfd for convenience (no need to specify hugetlb
> mem-path for ex), without the share memory aspects.
ok, acked.

btw:
looking at memfd_backend_memory_alloc/file_backend_memory_alloc
both needlessly are are calling host_memory_backend_mr_inited()
which creates an illusion that alloc could be called multiple times
but it isn't, could you clean that up?

In addition a following error path needs fixing
memfd_backend_memory_alloc()
     if (fd == -1) {                                                              
        return;                                                                  
    }
it must set errp

> 
> (but the patch was mostly for completeness)
> 
> >  
> > >  
> > > >  
> > > > >  
> > > > > >
> > > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > > > > ---
> > > > > >  backends/hostmem-memfd.c | 4 +++-
> > > > > >  qemu-options.hx          | 4 +++-
> > > > > >  2 files changed, 6 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
> > > > > > index 1e20fe0ba8..abd52364db 100644
> > > > > > --- a/backends/hostmem-memfd.c
> > > > > > +++ b/backends/hostmem-memfd.c
> > > > > > @@ -59,7 +59,8 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
> > > > > >
> > > > > >      name = object_get_canonical_path(OBJECT(backend));
> > > > > >      memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend),
> > > > > > -                                   name, backend->size, true, fd, errp);
> > > > > > +                                   name, backend->size,
> > > > > > +                                   backend->share, fd, errp);
> > > > > >      g_free(name);
> > > > > >  }
> > > > > >
> > > > > > @@ -131,6 +132,7 @@ memfd_backend_instance_init(Object *obj)
> > > > > >
> > > > > >      /* default to sealed file */
> > > > > >      m->seal = true;
> > > > > > +    MEMORY_BACKEND(m)->share = true;
> > > > > >  }
> > > > > >
> > > > > >  static void
> > > > > > diff --git a/qemu-options.hx b/qemu-options.hx
> > > > > > index f14e423b7c..25c02d72b8 100644
> > > > > > --- a/qemu-options.hx
> > > > > > +++ b/qemu-options.hx
> > > > > > @@ -4029,7 +4029,7 @@ Memory backend objects offer more control than the @option{-m} option that is
> > > > > >  traditionally used to define guest RAM. Please refer to
> > > > > >  @option{memory-backend-file} for a description of the options.
> > > > > >
> > > > > > -@item -object memory-backend-memfd,id=@var{id},merge=@var{on|off},dump=@var{on|off},prealloc=@var{on|off},size=@var{size},host-nodes=@var{host-nodes},policy=@var{default|preferred|bind|interleave},seal=@var{on|off},hugetlb=@var{on|off},hugetlbsize=@var{size}
> > > > > > +@item -object memory-backend-memfd,id=@var{id},merge=@var{on|off},dump=@var{on|off},share=@var{on|off},prealloc=@var{on|off},size=@var{size},host-nodes=@var{host-nodes},policy=@var{default|preferred|bind|interleave},seal=@var{on|off},hugetlb=@var{on|off},hugetlbsize=@var{size}
> > > > > >
> > > > > >  Creates an anonymous memory file backend object, which allows QEMU to
> > > > > >  share the memory with an external process (e.g. when using
> > > > > > @@ -4051,6 +4051,8 @@ with the @option{seal} option (requires at least Linux 4.16).
> > > > > >  Please refer to @option{memory-backend-file} for a description of the
> > > > > >  other options.
> > > > > >
> > > > > > +The @option{share} boolean option is @var{on} by default with memfd.
> > > > > > +
> > > > > >  @item -object rng-random,id=@var{id},filename=@var{/dev/random}
> > > > > >
> > > > > >  Creates a random number generator backend which obtains entropy from  
> > > > >
> > > > >  
> > > >
> > > >
> > > > --
> > > > Marc-André Lureau  
> > >
> > >
> > >  
> >  
> 
> 

Re: [Qemu-devel] [PATCH] hostmem-memfd: honour share=on/off property

[Marc-André Lureau]

Hi

On Thu, Aug 30, 2018 at 10:20 AM Igor Mammedov <imammedo@redhat.com> wrote:
>
> On Wed, 29 Aug 2018 14:36:00 +0200
> Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
>
> > Hi
> >
> > On Wed, Aug 29, 2018 at 10:50 AM Igor Mammedov <imammedo@redhat.com> wrote:
> > >
> > > On Tue, 28 Aug 2018 18:55:35 +0200
> > > Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
> > >
> > > > On Tue, Aug 28, 2018 at 6:54 PM Marc-André Lureau
> > > > <marcandre.lureau@gmail.com> wrote:
> > > > >
> > > > > Hi
> > > > >
> > > > > On Tue, Aug 28, 2018 at 6:49 PM Igor Mammedov <imammedo@redhat.com> wrote:
> > > > > >
> > > > > > On Tue, 28 Aug 2018 17:38:40 +0200
> > > > > > Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
> > > > > >
> > > > > > > The share=on/off property is used to modified mmap() MAP_SHARED
> > > > > > > setting. Make it on by default for convenience and compatibility
> > > > > > > reasons.
> > > > > > it would be nice to state reasons here.
> > > > > >
> > > > > > also flipping default to on is behavioral change when
> > > > > > used with bind policy which could make backend initialization fail
> > > > > > so CLI that used to work (if there are any users of this without shared=on)
> > > > > > won't work anymore.
> > > > >
> > > > > memfd backend is passing shared=true to memory_region_init_ram_from_fd().
> > > > >
> > > > > The patch allows to tweak that (instead of ignoring the property),
> > > > > while keeping the value to shared=true.
> > > >
> > > > keeping the default value to shared=trrue.
> > > ok,
> > > another question:
> > >  does it make sense to allow "shared=false" at all?
> > >  should we bail out if user asks for it?
> >
> > You may want to use memfd for convenience (no need to specify hugetlb
> > mem-path for ex), without the share memory aspects.
> ok, acked.
>
> btw:
> looking at memfd_backend_memory_alloc/file_backend_memory_alloc
> both needlessly are are calling host_memory_backend_mr_inited()
> which creates an illusion that alloc could be called multiple times
> but it isn't, could you clean that up?

ok

>
> In addition a following error path needs fixing
> memfd_backend_memory_alloc()
>      if (fd == -1) {
>         return;
>     }
> it must set errp

The error is already set by qemu_memfd_create()

>
> >
> > (but the patch was mostly for completeness)
> >
> > >
> > > >
> > > > >
> > > > > >
> > > > > > >
> > > > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > > > > > ---
> > > > > > >  backends/hostmem-memfd.c | 4 +++-
> > > > > > >  qemu-options.hx          | 4 +++-
> > > > > > >  2 files changed, 6 insertions(+), 2 deletions(-)
> > > > > > >
> > > > > > > diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
> > > > > > > index 1e20fe0ba8..abd52364db 100644
> > > > > > > --- a/backends/hostmem-memfd.c
> > > > > > > +++ b/backends/hostmem-memfd.c
> > > > > > > @@ -59,7 +59,8 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
> > > > > > >
> > > > > > >      name = object_get_canonical_path(OBJECT(backend));
> > > > > > >      memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend),
> > > > > > > -                                   name, backend->size, true, fd, errp);
> > > > > > > +                                   name, backend->size,
> > > > > > > +                                   backend->share, fd, errp);
> > > > > > >      g_free(name);
> > > > > > >  }
> > > > > > >
> > > > > > > @@ -131,6 +132,7 @@ memfd_backend_instance_init(Object *obj)
> > > > > > >
> > > > > > >      /* default to sealed file */
> > > > > > >      m->seal = true;
> > > > > > > +    MEMORY_BACKEND(m)->share = true;
> > > > > > >  }
> > > > > > >
> > > > > > >  static void
> > > > > > > diff --git a/qemu-options.hx b/qemu-options.hx
> > > > > > > index f14e423b7c..25c02d72b8 100644
> > > > > > > --- a/qemu-options.hx
> > > > > > > +++ b/qemu-options.hx
> > > > > > > @@ -4029,7 +4029,7 @@ Memory backend objects offer more control than the @option{-m} option that is
> > > > > > >  traditionally used to define guest RAM. Please refer to
> > > > > > >  @option{memory-backend-file} for a description of the options.
> > > > > > >
> > > > > > > -@item -object memory-backend-memfd,id=@var{id},merge=@var{on|off},dump=@var{on|off},prealloc=@var{on|off},size=@var{size},host-nodes=@var{host-nodes},policy=@var{default|preferred|bind|interleave},seal=@var{on|off},hugetlb=@var{on|off},hugetlbsize=@var{size}
> > > > > > > +@item -object memory-backend-memfd,id=@var{id},merge=@var{on|off},dump=@var{on|off},share=@var{on|off},prealloc=@var{on|off},size=@var{size},host-nodes=@var{host-nodes},policy=@var{default|preferred|bind|interleave},seal=@var{on|off},hugetlb=@var{on|off},hugetlbsize=@var{size}
> > > > > > >
> > > > > > >  Creates an anonymous memory file backend object, which allows QEMU to
> > > > > > >  share the memory with an external process (e.g. when using
> > > > > > > @@ -4051,6 +4051,8 @@ with the @option{seal} option (requires at least Linux 4.16).
> > > > > > >  Please refer to @option{memory-backend-file} for a description of the
> > > > > > >  other options.
> > > > > > >
> > > > > > > +The @option{share} boolean option is @var{on} by default with memfd.
> > > > > > > +
> > > > > > >  @item -object rng-random,id=@var{id},filename=@var{/dev/random}
> > > > > > >
> > > > > > >  Creates a random number generator backend which obtains entropy from
> > > > > >
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Marc-André Lureau
> > > >
> > > >
> > > >
> > >
> >
> >
>

thanks

-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH] hostmem-memfd: honour share=on/off property

[Paolo Bonzini]

On 28/08/18 17:38, Marc-André Lureau wrote:
> The share=on/off property is used to modified mmap() MAP_SHARED
> setting. Make it on by default for convenience and compatibility
> reasons.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Is this still needed?

Paolo

> ---
>  backends/hostmem-memfd.c | 4 +++-
>  qemu-options.hx          | 4 +++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
> index 1e20fe0ba8..abd52364db 100644
> --- a/backends/hostmem-memfd.c
> +++ b/backends/hostmem-memfd.c
> @@ -59,7 +59,8 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>  
>      name = object_get_canonical_path(OBJECT(backend));
>      memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend),
> -                                   name, backend->size, true, fd, errp);
> +                                   name, backend->size,
> +                                   backend->share, fd, errp);
>      g_free(name);
>  }
>  
> @@ -131,6 +132,7 @@ memfd_backend_instance_init(Object *obj)
>  
>      /* default to sealed file */
>      m->seal = true;
> +    MEMORY_BACKEND(m)->share = true;
>  }
>  
>  static void
> diff --git a/qemu-options.hx b/qemu-options.hx
> index f14e423b7c..25c02d72b8 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4029,7 +4029,7 @@ Memory backend objects offer more control than the @option{-m} option that is
>  traditionally used to define guest RAM. Please refer to
>  @option{memory-backend-file} for a description of the options.
>  
> -@item -object memory-backend-memfd,id=@var{id},merge=@var{on|off},dump=@var{on|off},prealloc=@var{on|off},size=@var{size},host-nodes=@var{host-nodes},policy=@var{default|preferred|bind|interleave},seal=@var{on|off},hugetlb=@var{on|off},hugetlbsize=@var{size}
> +@item -object memory-backend-memfd,id=@var{id},merge=@var{on|off},dump=@var{on|off},share=@var{on|off},prealloc=@var{on|off},size=@var{size},host-nodes=@var{host-nodes},policy=@var{default|preferred|bind|interleave},seal=@var{on|off},hugetlb=@var{on|off},hugetlbsize=@var{size}
>  
>  Creates an anonymous memory file backend object, which allows QEMU to
>  share the memory with an external process (e.g. when using
> @@ -4051,6 +4051,8 @@ with the @option{seal} option (requires at least Linux 4.16).
>  Please refer to @option{memory-backend-file} for a description of the
>  other options.
>  
> +The @option{share} boolean option is @var{on} by default with memfd.
> +
>  @item -object rng-random,id=@var{id},filename=@var{/dev/random}
>  
>  Creates a random number generator backend which obtains entropy from
>