Re: [Qemu-devel] [PATCH 2/2] Debug : Add error messages before a call to debug().

[Prerna]

Hi Eric,

Thank you for the review.

On Wed, Apr 27, 2016 at 9:30 PM, Eric Blake <address@hidden> wrote:

On 04/14/2016 09:02 PM, Prerna Saxena wrote:
> Qemu code has abort() calls in various places which raises a SIGABRT;
> This patch adds error messages before (most)calls to abort(), so that
> it is easier to determine why QEMU died.

The subject line says you are adding messages before debug(), but the
rest of the patch is adding message before abort(). You'll need to fix
that.  Also, subject lines usually don't end in '.'

Sorry, the subject line shouldve said : "Add error messages before abort()".

Will resend.

  

> +++ b/block.c
> @@ -3725,6 +3725,7 @@ void bdrv_remove_aio_context_notifier(BlockDriverState *bs,
>          }
>      }
>
> +    error_report("Matching context notifier not found for removal. Aborting");
>      abort();

The "Aborting" part of the message is redundant; it's pretty obvious
that qemu aborted.

Agree.

 

I also wonder if you should be using g_assert_not_reached() instead of
abort() in some (all?) of the places touched in this patch - at which
point you don't have to worry about inventing a message that will never
be printed.  The reason I suggest it is that g_assert_not_reached() is
self-documenting, and prints a nicer message than abort() if it does
accidentally get reached.

Ah, thanks for this suggestion. I will look up g_assert_not_reached() to see how I can use it (at all places, possibly).

Regards,

Prerna

 

--
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org