Re: [PATCH] kvm: turn fprintf into error_report

[Markus Armbruster <armbru@redhat.com>]

Quick drive-by comments...

Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> writes:

> Main benefit: ability to print timestamps.

Possible additional benefits:

* Ability to print guest name

* Print program name

* "This is an error" is obvious even at a glance

Benefits not applicable here (I believe):

* Print location if we have one

* Automatic redirection to the human monitor when appropriate

> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  accel/kvm/kvm-all.c | 64 ++++++++++++++++++++++-----------------------
>  1 file changed, 31 insertions(+), 33 deletions(-)
>
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 83cbd120a84..d271699db87 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -300,7 +300,7 @@ static KVMSlot *kvm_alloc_slot(KVMMemoryListener *kml)
       KVMSlot *slot = kvm_get_free_slot(kml);

       if (slot) {
>          return slot;
>      }
>  
> -    fprintf(stderr, "%s: no free slot available\n", __func__);
> +    error_report("%s: no free slot available", __func__);
>      abort();
>  }
>  

Bah, too much ceremony for me.  I'd do

       KVMSlot *slot = kvm_get_free_slot(kml);

       assert(slot);
       return slot;

The "print something to help developers fix the programming error;
abort()" pattern leads to having __func__ in error messages, which is
normally an anti-pattern, because error messages are for users.

But all that is outside the scope of your patch, and the maintainers may
have other ideas anyway.

[...]