Re: [Qemu-devel] [PATCH] deprecate -mem-path fallback to anonymous RAM

[Markus Armbruster <armbru@redhat.com>]

Igor Mammedov <imammedo@redhat.com> writes:

> On Mon, 24 Jun 2019 10:17:33 +0200
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> Igor Mammedov <imammedo@redhat.com> writes:
>> 
>> > Fallback might affect guest or worse whole host performance
>> > or functionality if backing file were used to share guest RAM
>> > with another process.
>> >
>> > Patch deprecates fallback so that we could remove it in future
>> > and ensure that QEMU will provide expected behavior and fail if
>> > it can't use user provided backing file.
>> >
>> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>> > ---
>> > PS:
>> > Patch is written on top of
>> >   [PATCH v4 0/3] numa: deprecate '-numa node,  mem' and default memory distribution
>> > to avoid conflicts in qemu-deprecated.texi
>> >
>> >  numa.c               | 4 ++--
>> >  qemu-deprecated.texi | 8 ++++++++
>> >  2 files changed, 10 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/numa.c b/numa.c
>> > index 91a29138a2..53d67b8ad9 100644
>> > --- a/numa.c
>> > +++ b/numa.c
>> > @@ -494,8 +494,8 @@ static void allocate_system_memory_nonnuma(MemoryRegion *mr, Object *owner,
>> >              if (mem_prealloc) {
>> >                  exit(1);
>> >              }
>> > -            error_report("falling back to regular RAM allocation.");
>> > -
>> > +            warn_report("falling back to regular RAM allocation. "
>> > +                        "Fallback to RAM allocation is deprecated.");  
>> 
>> Can we give the user clues on how to avoid the deprecated fallback?
>
> I've intentionally left it out for a lack of clear enough advise.
> Something like:
>   "Make sure that host has resources to map file pointed by -mem-path"
> would be pretty useless.

I see.

> I think describing how host should be configured in various ways
> depending on type of backing storage is well out of scope of any
> QEMU documentation. But if you have an idea to what to put there
> (or what to put in deprecation doc and refer to from here),
> I'll add it on respin.
>
>> Warning message nitpick: the message should be a single phrase, with no
>> newline or trailing punctuation.  Suggest something like
>> 
>>                warn_report("falling back to regular RAM allocation");
>>                error_printf("This is deprecated.  <Advice on what\n"
>>                             "to do goes here>\n");
>> 
>> >              /* Legacy behavior: if allocation failed, fall back to
>> >               * regular RAM allocation.
>> >               */
>> > diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
>> > index 2fe9b72121..2193705644 100644
>> > --- a/qemu-deprecated.texi
>> > +++ b/qemu-deprecated.texi
>> > @@ -112,6 +112,14 @@ QEMU using implicit generic or board specific splitting rule.
>> >  Use @option{memdev} with @var{memory-backend-ram} backend or @option{mem} (if
>> >  it's supported by used machine type) to define mapping explictly instead.
>> >  
>> > +@subsection -mem-path fallback to RAM (since 4.1)
>> > +Currently if system memory allocation from file pointed by @option{mem-path}
>> > +fails, QEMU fallbacks to allocating from anonymous RAM. Which might result
>> > +in unpredictable behavior since provided backing file wasn't used.  
>> 
>> 
>> Noch such verb "to fallback", obvious fix "QEMU falls back to"
>> 
>> Suggest "RAM, which might".
>> 
>> Better: "since the backing file specified by the user is ignored".
>> 
>> >                                                                     In future
>> > +QEMU will not fallback and fail to start up, so user could fix his/her QEMU/host
>> > +configuration or explicitly use -m without -mem-path if system memory allocated
>> > +from anonymous RAM suits usecase.  
>> 
>> What's "system memory allocation"?
> Using man page language, would be
>  'guest startup RAM size'
> acceptable?
>
>
>> Perhaps: "In the future, QEMU will not fall back, but fail instead.
>> Adjust either the host configuration [FIXME how?] or the QEMU
>> configuration [FIXME how?]."
>
> Maybe
> "
>  In the future, QEMU will not fall back, but fail instead.
>  Adjust either the QEMU configuration by removing @option{-mem-path} so
>  QEMU will use only anonymous or host configuration to make sure that

Do you mean "only anonymous memory"?

>  there are sufficient resources on backing storage pointed by -mem-path
>  to allocate amount specified by @option{-m}.
> "

Perhaps this could suffice: "Users will be responsible for making sure
the backing storage specified with -mem-path can actually provide the
guest RAM configured with -m."

>> > +
>> >  @section QEMU Machine Protocol (QMP) commands
>> >  
>> >  @subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0)  
>>