* Re: [Qemu-devel] [PATCH] hostmem-memfd: honour share=on/off property
2018-08-28 15:38 [Qemu-devel] [PATCH] hostmem-memfd: honour share=on/off property Marc-André Lureau
@ 2018-08-28 16:43 ` Igor Mammedov
2018-08-28 16:54 ` Marc-André Lureau
2018-08-30 8:14 ` Igor Mammedov
2018-11-26 16:57 ` Paolo Bonzini
2 siblings, 1 reply; 10+ messages in thread
From: Igor Mammedov @ 2018-08-28 16:43 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: qemu-devel, pbonzini
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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH] hostmem-memfd: honour share=on/off property
2018-08-28 16:43 ` Igor Mammedov
@ 2018-08-28 16:54 ` Marc-André Lureau
2018-08-28 16:55 ` Marc-André Lureau
0 siblings, 1 reply; 10+ messages in thread
From: Marc-André Lureau @ 2018-08-28 16:54 UTC (permalink / raw)
To: Igor Mammedov; +Cc: Paolo Bonzini, QEMU
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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH] hostmem-memfd: honour share=on/off property
2018-08-28 16:54 ` Marc-André Lureau
@ 2018-08-28 16:55 ` Marc-André Lureau
2018-08-29 8:50 ` Igor Mammedov
0 siblings, 1 reply; 10+ messages in thread
From: Marc-André Lureau @ 2018-08-28 16:55 UTC (permalink / raw)
To: Igor Mammedov; +Cc: Paolo Bonzini, QEMU
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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH] hostmem-memfd: honour share=on/off property
2018-08-28 16:55 ` Marc-André Lureau
@ 2018-08-29 8:50 ` Igor Mammedov
2018-08-29 12:36 ` Marc-André Lureau
0 siblings, 1 reply; 10+ messages in thread
From: Igor Mammedov @ 2018-08-29 8:50 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: Paolo Bonzini, QEMU
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
>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH] hostmem-memfd: honour share=on/off property
2018-08-29 8:50 ` Igor Mammedov
@ 2018-08-29 12:36 ` Marc-André Lureau
2018-08-30 8:20 ` Igor Mammedov
0 siblings, 1 reply; 10+ messages in thread
From: Marc-André Lureau @ 2018-08-29 12:36 UTC (permalink / raw)
To: Igor Mammedov; +Cc: Paolo Bonzini, QEMU
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
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH] hostmem-memfd: honour share=on/off property
2018-08-29 12:36 ` Marc-André Lureau
@ 2018-08-30 8:20 ` Igor Mammedov
2018-08-30 17:10 ` Marc-André Lureau
0 siblings, 1 reply; 10+ messages in thread
From: Igor Mammedov @ 2018-08-30 8:20 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: Paolo Bonzini, QEMU
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
> > >
> > >
> > >
> >
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH] hostmem-memfd: honour share=on/off property
2018-08-30 8:20 ` Igor Mammedov
@ 2018-08-30 17:10 ` Marc-André Lureau
0 siblings, 0 replies; 10+ messages in thread
From: Marc-André Lureau @ 2018-08-30 17:10 UTC (permalink / raw)
To: Igor Mammedov; +Cc: Paolo Bonzini, QEMU
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PATCH] hostmem-memfd: honour share=on/off property
2018-08-28 15:38 [Qemu-devel] [PATCH] hostmem-memfd: honour share=on/off property Marc-André Lureau
2018-08-28 16:43 ` Igor Mammedov
@ 2018-08-30 8:14 ` Igor Mammedov
2018-11-26 16:57 ` Paolo Bonzini
2 siblings, 0 replies; 10+ messages in thread
From: Igor Mammedov @ 2018-08-30 8:14 UTC (permalink / raw)
To: Marc-André Lureau; +Cc: qemu-devel, pbonzini
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>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [Qemu-devel] [PATCH] hostmem-memfd: honour share=on/off property
2018-08-28 15:38 [Qemu-devel] [PATCH] hostmem-memfd: honour share=on/off property Marc-André Lureau
2018-08-28 16:43 ` Igor Mammedov
2018-08-30 8:14 ` Igor Mammedov
@ 2018-11-26 16:57 ` Paolo Bonzini
2 siblings, 0 replies; 10+ messages in thread
From: Paolo Bonzini @ 2018-11-26 16:57 UTC (permalink / raw)
To: Marc-André Lureau, qemu-devel
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
>
^ permalink raw reply [flat|nested] 10+ messages in thread