[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] hostmem: no need to check for host_memory_backe
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-devel] [PATCH] hostmem: no need to check for host_memory_backend_mr_inited() in alloc() |
Date: |
Fri, 31 Aug 2018 08:53:35 +0200 |
On Thu, 30 Aug 2018 16:09:58 -0300
Eduardo Habkost <address@hidden> wrote:
> On Thu, Aug 30, 2018 at 07:50:19PM +0200, Marc-André Lureau wrote:
> > 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, it's
> > called once from UserCreatable complete().
could you extend this patch to cover file_backend_memory_alloc() as well?
Otherwise this logic would be copy-pasted again in the future.
> > Suggested-by: Igor Mammedov <address@hidden>
> > Signed-off-by: Marc-André Lureau <address@hidden>
>
> If calling the function multiple times is a mistake, should we
> replace the checks with:
> assert(!host_memory_backend_mr_inited(backend));
> ?
it is confusing calling it where it's not necessary, hence this cleanup.
Considering that HostMemoryBackendClass::alloc is internal bussines
of hostmem backends, assert() would just add clutter.
> > ---
> > backends/hostmem-file.c | 2 +-
> > backends/hostmem-memfd.c | 4 ----
> > 2 files changed, 1 insertion(+), 5 deletions(-)
> >
> > diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> > index 2476dcb435..5cd5fa75a7 100644
> > --- a/backends/hostmem-file.c
> > +++ b/backends/hostmem-file.c
> > @@ -54,7 +54,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend,
> > Error **errp)
> > #ifndef CONFIG_LINUX
> > error_setg(errp, "-mem-path not supported on this host");
> > #else
> > - if (!host_memory_backend_mr_inited(backend)) {
> > + {
> > gchar *path;
> > backend->force_prealloc = mem_prealloc;
> > path = object_get_canonical_path(OBJECT(backend));
> > diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
> > index abd52364db..7184918112 100644
> > --- a/backends/hostmem-memfd.c
> > +++ b/backends/hostmem-memfd.c
> > @@ -44,10 +44,6 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend,
> > Error **errp)
> > return;
> > }
> >
> > - if (host_memory_backend_mr_inited(backend)) {
> > - return;
> > - }
> > -
> > backend->force_prealloc = mem_prealloc;
> > fd = qemu_memfd_create(TYPE_MEMORY_BACKEND_MEMFD, backend->size,
> > m->hugetlb, m->hugetlbsize, m->seal ?
> > --
> > 2.19.0.rc0.48.gb9dfa238d5
> >
>