If memory allocation fails when using -mem-path, QEMU is supposed to print
out a message to indicate that fallback to anonymous RAM is deprecated. This
is done with error_printf() which does output buffering. As a consequence,
the message is only printed at the next flush, eg. when quiting QEMU, and
it also lacks a trailing newline:

qemu-system-ppc64: unable to map backing store for guest RAM: Cannot allocate memory
qemu-system-ppc64: warning: falling back to regular RAM allocation
QEMU 4.1.50 monitor - type 'help' for more information
(qemu) q
This is deprecated. Make sure that -mem-path  specified path has sufficient resources to allocate -m specified RAM amountgreg@boss02:~/Work/qemu/qemu-spapr$

Add the missing \n to fix both issues.

Fixes: cb79224b7e4b "deprecate -mem-path fallback to anonymous RAM"
Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/core/numa.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/core/numa.c b/hw/core/numa.c
index 038c96d4abc6..e3332a984f7c 100644
--- a/hw/core/numa.c
+++ b/hw/core/numa.c
@@ -503,7 +503,7 @@ static void allocate_system_memory_nonnuma(MemoryRegion *mr, Object *owner,
             warn_report("falling back to regular RAM allocation");
             error_printf("This is deprecated. Make sure that -mem-path "
                          " specified path has sufficient resources to allocate"
-                         " -m specified RAM amount");
+                         " -m specified RAM amount\n");
             /* Legacy behavior: if allocation failed, fall back to
              * regular RAM allocation.
              */

Oops I hadn't realized that Marcel had a new e-mail address... since last
year :-\ Sorry for the noise.

On Wed, 06 Nov 2019 13:46:40 +0100
Greg Kurz <groug@kaod.org> wrote:

> If memory allocation fails when using -mem-path, QEMU is supposed to print
> out a message to indicate that fallback to anonymous RAM is deprecated. This
> is done with error_printf() which does output buffering. As a consequence,
> the message is only printed at the next flush, eg. when quiting QEMU, and
> it also lacks a trailing newline:
> 
> qemu-system-ppc64: unable to map backing store for guest RAM: Cannot allocate memory
> qemu-system-ppc64: warning: falling back to regular RAM allocation
> QEMU 4.1.50 monitor - type 'help' for more information
> (qemu) q
> This is deprecated. Make sure that -mem-path  specified path has sufficient resources to allocate -m specified RAM amountgreg@boss02:~/Work/qemu/qemu-spapr$
> 
> Add the missing \n to fix both issues.
> 
> Fixes: cb79224b7e4b "deprecate -mem-path fallback to anonymous RAM"
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
>  hw/core/numa.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/core/numa.c b/hw/core/numa.c
> index 038c96d4abc6..e3332a984f7c 100644
> --- a/hw/core/numa.c
> +++ b/hw/core/numa.c
> @@ -503,7 +503,7 @@ static void allocate_system_memory_nonnuma(MemoryRegion *mr, Object *owner,
>              warn_report("falling back to regular RAM allocation");
>              error_printf("This is deprecated. Make sure that -mem-path "
>                           " specified path has sufficient resources to allocate"
> -                         " -m specified RAM amount");
> +                         " -m specified RAM amount\n");
>              /* Legacy behavior: if allocation failed, fall back to
>               * regular RAM allocation.
>               */
> 
> 

Le 06/11/2019 à 13:46, Greg Kurz a écrit :
> If memory allocation fails when using -mem-path, QEMU is supposed to print
> out a message to indicate that fallback to anonymous RAM is deprecated. This
> is done with error_printf() which does output buffering. As a consequence,
> the message is only printed at the next flush, eg. when quiting QEMU, and
> it also lacks a trailing newline:
> 
> qemu-system-ppc64: unable to map backing store for guest RAM: Cannot allocate memory
> qemu-system-ppc64: warning: falling back to regular RAM allocation
> QEMU 4.1.50 monitor - type 'help' for more information
> (qemu) q
> This is deprecated. Make sure that -mem-path  specified path has sufficient resources to allocate -m specified RAM amountgreg@boss02:~/Work/qemu/qemu-spapr$
> 
> Add the missing \n to fix both issues.
> 
> Fixes: cb79224b7e4b "deprecate -mem-path fallback to anonymous RAM"
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
>  hw/core/numa.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/core/numa.c b/hw/core/numa.c
> index 038c96d4abc6..e3332a984f7c 100644
> --- a/hw/core/numa.c
> +++ b/hw/core/numa.c
> @@ -503,7 +503,7 @@ static void allocate_system_memory_nonnuma(MemoryRegion *mr, Object *owner,
>              warn_report("falling back to regular RAM allocation");
>              error_printf("This is deprecated. Make sure that -mem-path "
>                           " specified path has sufficient resources to allocate"
> -                         " -m specified RAM amount");
> +                         " -m specified RAM amount\n");
>              /* Legacy behavior: if allocation failed, fall back to
>               * regular RAM allocation.
>               */
> 
> 

Why is this an error_printf() and not an error_report()?

Thanks,
Laurent

On Wed, 6 Nov 2019 14:01:01 +0100
Laurent Vivier <laurent@vivier.eu> wrote:

> Le 06/11/2019 à 13:46, Greg Kurz a écrit :
> > If memory allocation fails when using -mem-path, QEMU is supposed to print
> > out a message to indicate that fallback to anonymous RAM is deprecated. This
> > is done with error_printf() which does output buffering. As a consequence,
> > the message is only printed at the next flush, eg. when quiting QEMU, and
> > it also lacks a trailing newline:
> > 
> > qemu-system-ppc64: unable to map backing store for guest RAM: Cannot allocate memory
> > qemu-system-ppc64: warning: falling back to regular RAM allocation
> > QEMU 4.1.50 monitor - type 'help' for more information
> > (qemu) q
> > This is deprecated. Make sure that -mem-path  specified path has sufficient resources to allocate -m specified RAM amountgreg@boss02:~/Work/qemu/qemu-spapr$
> > 
> > Add the missing \n to fix both issues.
> > 
> > Fixes: cb79224b7e4b "deprecate -mem-path fallback to anonymous RAM"
> > Signed-off-by: Greg Kurz <groug@kaod.org>
> > ---
> >  hw/core/numa.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/hw/core/numa.c b/hw/core/numa.c
> > index 038c96d4abc6..e3332a984f7c 100644
> > --- a/hw/core/numa.c
> > +++ b/hw/core/numa.c
> > @@ -503,7 +503,7 @@ static void allocate_system_memory_nonnuma(MemoryRegion *mr, Object *owner,
> >              warn_report("falling back to regular RAM allocation");
> >              error_printf("This is deprecated. Make sure that -mem-path "
> >                           " specified path has sufficient resources to allocate"
> > -                         " -m specified RAM amount");
> > +                         " -m specified RAM amount\n");
> >              /* Legacy behavior: if allocation failed, fall back to
> >               * regular RAM allocation.
> >               */
> > 
> > 
> 
> Why is this an error_printf() and not an error_report()?
> 

Because CODING_STYLE suggests to do so I guess:

Reporting errors to the human user
----------------------------------

Do not use printf(), fprintf() or monitor_printf().  Instead, use
error_report() or error_vreport() from error-report.h.  This ensures the
error is reported in the right place (current monitor or stderr), and in
a uniform format.

Use error_printf() & friends to print additional information. <===

error_report() prints the current location.  In certain common cases
like command line parsing, the current location is tracked
automatically.  To manipulate it manually, use the loc_``*``() from
error-report.h.

> Thanks,
> Laurent
> 

Greg Kurz <groug@kaod.org> writes:

> On Wed, 6 Nov 2019 14:01:01 +0100
> Laurent Vivier <laurent@vivier.eu> wrote:
>
>> Le 06/11/2019 à 13:46, Greg Kurz a écrit :
>> > If memory allocation fails when using -mem-path, QEMU is supposed to print
>> > out a message to indicate that fallback to anonymous RAM is deprecated. This
>> > is done with error_printf() which does output buffering. As a consequence,
>> > the message is only printed at the next flush, eg. when quiting QEMU, and
>> > it also lacks a trailing newline:
>> > 
>> > qemu-system-ppc64: unable to map backing store for guest RAM: Cannot allocate memory
>> > qemu-system-ppc64: warning: falling back to regular RAM allocation
>> > QEMU 4.1.50 monitor - type 'help' for more information
>> > (qemu) q
>> > This is deprecated. Make sure that -mem-path  specified path has sufficient resources to allocate -m specified RAM amountgreg@boss02:~/Work/qemu/qemu-spapr$
>> > 
>> > Add the missing \n to fix both issues.
>> > 
>> > Fixes: cb79224b7e4b "deprecate -mem-path fallback to anonymous RAM"
>> > Signed-off-by: Greg Kurz <groug@kaod.org>
>> > ---
>> >  hw/core/numa.c |    2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> > 
>> > diff --git a/hw/core/numa.c b/hw/core/numa.c
>> > index 038c96d4abc6..e3332a984f7c 100644
>> > --- a/hw/core/numa.c
>> > +++ b/hw/core/numa.c
>> > @@ -503,7 +503,7 @@ static void allocate_system_memory_nonnuma(MemoryRegion *mr, Object *owner,
>> >              warn_report("falling back to regular RAM allocation");
>> >              error_printf("This is deprecated. Make sure that -mem-path "
>> >                           " specified path has sufficient resources to allocate"
>> > -                         " -m specified RAM amount");
>> > +                         " -m specified RAM amount\n");
>> >              /* Legacy behavior: if allocation failed, fall back to
>> >               * regular RAM allocation.
>> >               */
>> > 
>> > 
>> 
>> Why is this an error_printf() and not an error_report()?
>> 
>
> Because CODING_STYLE suggests to do so I guess:
>
> Reporting errors to the human user
> ----------------------------------
>
> Do not use printf(), fprintf() or monitor_printf().  Instead, use
> error_report() or error_vreport() from error-report.h.  This ensures the
> error is reported in the right place (current monitor or stderr), and in
> a uniform format.
>
> Use error_printf() & friends to print additional information. <===

You're right.

Since I have nothing queued up right now, I'd prefer to have this go via
qemu-trivial.

> error_report() prints the current location.  In certain common cases
> like command line parsing, the current location is tracked
> automatically.  To manipulate it manually, use the loc_``*``() from
> error-report.h.

Le 06/11/2019 à 17:55, Markus Armbruster a écrit :
> Greg Kurz <groug@kaod.org> writes:
> 
>> On Wed, 6 Nov 2019 14:01:01 +0100
>> Laurent Vivier <laurent@vivier.eu> wrote:
>>
>>> Le 06/11/2019 à 13:46, Greg Kurz a écrit :
>>>> If memory allocation fails when using -mem-path, QEMU is supposed to print
>>>> out a message to indicate that fallback to anonymous RAM is deprecated. This
>>>> is done with error_printf() which does output buffering. As a consequence,
>>>> the message is only printed at the next flush, eg. when quiting QEMU, and
>>>> it also lacks a trailing newline:
>>>>
>>>> qemu-system-ppc64: unable to map backing store for guest RAM: Cannot allocate memory
>>>> qemu-system-ppc64: warning: falling back to regular RAM allocation
>>>> QEMU 4.1.50 monitor - type 'help' for more information
>>>> (qemu) q
>>>> This is deprecated. Make sure that -mem-path  specified path has sufficient resources to allocate -m specified RAM amountgreg@boss02:~/Work/qemu/qemu-spapr$
>>>>
>>>> Add the missing \n to fix both issues.
>>>>
>>>> Fixes: cb79224b7e4b "deprecate -mem-path fallback to anonymous RAM"
>>>> Signed-off-by: Greg Kurz <groug@kaod.org>
>>>> ---
>>>>  hw/core/numa.c |    2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/hw/core/numa.c b/hw/core/numa.c
>>>> index 038c96d4abc6..e3332a984f7c 100644
>>>> --- a/hw/core/numa.c
>>>> +++ b/hw/core/numa.c
>>>> @@ -503,7 +503,7 @@ static void allocate_system_memory_nonnuma(MemoryRegion *mr, Object *owner,
>>>>              warn_report("falling back to regular RAM allocation");
>>>>              error_printf("This is deprecated. Make sure that -mem-path "
>>>>                           " specified path has sufficient resources to allocate"
>>>> -                         " -m specified RAM amount");
>>>> +                         " -m specified RAM amount\n");
>>>>              /* Legacy behavior: if allocation failed, fall back to
>>>>               * regular RAM allocation.
>>>>               */
>>>>
>>>>
>>>
>>> Why is this an error_printf() and not an error_report()?
>>>
>>
>> Because CODING_STYLE suggests to do so I guess:
>>
>> Reporting errors to the human user
>> ----------------------------------
>>
>> Do not use printf(), fprintf() or monitor_printf().  Instead, use
>> error_report() or error_vreport() from error-report.h.  This ensures the
>> error is reported in the right place (current monitor or stderr), and in
>> a uniform format.
>>
>> Use error_printf() & friends to print additional information. <===
> 
> You're right.
> 
> Since I have nothing queued up right now, I'd prefer to have this go via
> qemu-trivial.

Ok, I will take it in my next qemu-trivial pull request (with the other
Greg's patch).

Thanks,
Laurent

Le 06/11/2019 à 15:12, Greg Kurz a écrit :
> On Wed, 6 Nov 2019 14:01:01 +0100
> Laurent Vivier <laurent@vivier.eu> wrote:
> 
>> Le 06/11/2019 à 13:46, Greg Kurz a écrit :
>>> If memory allocation fails when using -mem-path, QEMU is supposed to print
>>> out a message to indicate that fallback to anonymous RAM is deprecated. This
>>> is done with error_printf() which does output buffering. As a consequence,
>>> the message is only printed at the next flush, eg. when quiting QEMU, and
>>> it also lacks a trailing newline:
>>>
>>> qemu-system-ppc64: unable to map backing store for guest RAM: Cannot allocate memory
>>> qemu-system-ppc64: warning: falling back to regular RAM allocation
>>> QEMU 4.1.50 monitor - type 'help' for more information
>>> (qemu) q
>>> This is deprecated. Make sure that -mem-path  specified path has sufficient resources to allocate -m specified RAM amountgreg@boss02:~/Work/qemu/qemu-spapr$
>>>
>>> Add the missing \n to fix both issues.
>>>
>>> Fixes: cb79224b7e4b "deprecate -mem-path fallback to anonymous RAM"
>>> Signed-off-by: Greg Kurz <groug@kaod.org>
>>> ---
>>>  hw/core/numa.c |    2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/hw/core/numa.c b/hw/core/numa.c
>>> index 038c96d4abc6..e3332a984f7c 100644
>>> --- a/hw/core/numa.c
>>> +++ b/hw/core/numa.c
>>> @@ -503,7 +503,7 @@ static void allocate_system_memory_nonnuma(MemoryRegion *mr, Object *owner,
>>>              warn_report("falling back to regular RAM allocation");
>>>              error_printf("This is deprecated. Make sure that -mem-path "
>>>                           " specified path has sufficient resources to allocate"
>>> -                         " -m specified RAM amount");
>>> +                         " -m specified RAM amount\n");
>>>              /* Legacy behavior: if allocation failed, fall back to
>>>               * regular RAM allocation.
>>>               */
>>>
>>>
>>
>> Why is this an error_printf() and not an error_report()?
>>
> 
> Because CODING_STYLE suggests to do so I guess:
> 
> Reporting errors to the human user
> ----------------------------------
> 
> Do not use printf(), fprintf() or monitor_printf().  Instead, use
> error_report() or error_vreport() from error-report.h.  This ensures the
> error is reported in the right place (current monitor or stderr), and in
> a uniform format.
> 
> Use error_printf() & friends to print additional information. <===
> 
> error_report() prints the current location.  In certain common cases
> like command line parsing, the current location is tracked
> automatically.  To manipulate it manually, use the loc_``*``() from
> error-report.h.

So I guess it's to not report the current location and the binary name .

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

On 11/6/19 3:20 PM, Laurent Vivier wrote:
> Le 06/11/2019 à 15:12, Greg Kurz a écrit :
>> On Wed, 6 Nov 2019 14:01:01 +0100
>> Laurent Vivier <laurent@vivier.eu> wrote:
>>
>>> Le 06/11/2019 à 13:46, Greg Kurz a écrit :
>>>> If memory allocation fails when using -mem-path, QEMU is supposed to print
>>>> out a message to indicate that fallback to anonymous RAM is deprecated. This
>>>> is done with error_printf() which does output buffering. As a consequence,
>>>> the message is only printed at the next flush, eg. when quiting QEMU, and
>>>> it also lacks a trailing newline:
>>>>
>>>> qemu-system-ppc64: unable to map backing store for guest RAM: Cannot allocate memory
>>>> qemu-system-ppc64: warning: falling back to regular RAM allocation
>>>> QEMU 4.1.50 monitor - type 'help' for more information
>>>> (qemu) q
>>>> This is deprecated. Make sure that -mem-path  specified path has sufficient resources to allocate -m specified RAM amountgreg@boss02:~/Work/qemu/qemu-spapr$
>>>>
>>>> Add the missing \n to fix both issues.
>>>>
>>>> Fixes: cb79224b7e4b "deprecate -mem-path fallback to anonymous RAM"
>>>> Signed-off-by: Greg Kurz <groug@kaod.org>
>>>> ---
>>>>   hw/core/numa.c |    2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/hw/core/numa.c b/hw/core/numa.c
>>>> index 038c96d4abc6..e3332a984f7c 100644
>>>> --- a/hw/core/numa.c
>>>> +++ b/hw/core/numa.c
>>>> @@ -503,7 +503,7 @@ static void allocate_system_memory_nonnuma(MemoryRegion *mr, Object *owner,
>>>>               warn_report("falling back to regular RAM allocation");
>>>>               error_printf("This is deprecated. Make sure that -mem-path "
>>>>                            " specified path has sufficient resources to allocate"
>>>> -                         " -m specified RAM amount");
>>>> +                         " -m specified RAM amount\n");
>>>>               /* Legacy behavior: if allocation failed, fall back to
>>>>                * regular RAM allocation.
>>>>                */
>>>>
>>>>
>>>
>>> Why is this an error_printf() and not an error_report()?
>>>
>>
>> Because CODING_STYLE suggests to do so I guess:
>>
>> Reporting errors to the human user
>> ----------------------------------
>>
>> Do not use printf(), fprintf() or monitor_printf().  Instead, use
>> error_report() or error_vreport() from error-report.h.  This ensures the
>> error is reported in the right place (current monitor or stderr), and in
>> a uniform format.
>>
>> Use error_printf() & friends to print additional information. <===
>>
>> error_report() prints the current location.  In certain common cases
>> like command line parsing, the current location is tracked
>> automatically.  To manipulate it manually, use the loc_``*``() from
>> error-report.h.
> 
> So I guess it's to not report the current location and the binary name .
> 
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Le 06/11/2019 à 13:46, Greg Kurz a écrit :
> If memory allocation fails when using -mem-path, QEMU is supposed to print
> out a message to indicate that fallback to anonymous RAM is deprecated. This
> is done with error_printf() which does output buffering. As a consequence,
> the message is only printed at the next flush, eg. when quiting QEMU, and
> it also lacks a trailing newline:
> 
> qemu-system-ppc64: unable to map backing store for guest RAM: Cannot allocate memory
> qemu-system-ppc64: warning: falling back to regular RAM allocation
> QEMU 4.1.50 monitor - type 'help' for more information
> (qemu) q
> This is deprecated. Make sure that -mem-path  specified path has sufficient resources to allocate -m specified RAM amountgreg@boss02:~/Work/qemu/qemu-spapr$
> 
> Add the missing \n to fix both issues.
> 
> Fixes: cb79224b7e4b "deprecate -mem-path fallback to anonymous RAM"
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
>  hw/core/numa.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/core/numa.c b/hw/core/numa.c
> index 038c96d4abc6..e3332a984f7c 100644
> --- a/hw/core/numa.c
> +++ b/hw/core/numa.c
> @@ -503,7 +503,7 @@ static void allocate_system_memory_nonnuma(MemoryRegion *mr, Object *owner,
>              warn_report("falling back to regular RAM allocation");
>              error_printf("This is deprecated. Make sure that -mem-path "
>                           " specified path has sufficient resources to allocate"
> -                         " -m specified RAM amount");
> +                         " -m specified RAM amount\n");
>              /* Legacy behavior: if allocation failed, fall back to
>               * regular RAM allocation.
>               */
> 
> 

Applied to my trivial-patches branch.

Thanks,
Laurent