qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: qemu-pr-helper -v suppresses errors, isn't that weird?


From: Markus Armbruster
Subject: Re: qemu-pr-helper -v suppresses errors, isn't that weird?
Date: Mon, 22 Jun 2020 10:28:49 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux)

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 18/06/20 07:32, Markus Armbruster wrote:
>> prh_co_entry() reports reports errors reading requests / writing
>> responses only when @verbose (command line -v); relevant code appended
>> for you convenience.
>> 
>> Sure these are *errors*?  The program recovers and continues, and this
>> is deemed normal enough to inform the user only when he specifically
>> asks for it.  Yet when we inform, we format it as an error.  Should we
>> tune it down to warnings?
>
> They are errors, but they're errors in the client rather than in
> qemu-pr-helper.c itself.

The error message makes it look like the error was in qemu-pr-helper.

Also, continuing after reporting an error as if nothing happened smells
suspiciously.  Even when it's not wrong, it's prone to catch a wary
reader's eye.

What do you think about something like

    warn_reportf_err(local_err, "client trouble: ");

Yes, "client trouble" isn't so hot, but I lack the understanding to do
better.

> Paolo
>
>> 
>> static void coroutine_fn prh_co_entry(void *opaque)
>> {
>>     [...]
>>     while (atomic_read(&state) == RUNNING) {
>>         [...]
>>         sz = prh_read_request(client, &req, &resp, &local_err);
>>         if (sz < 0) {
>>             break;
>>         }
>>         [...]
>>         if (prh_write_response(client, &req, &resp, &local_err) < 0) {
>>             break;
>>         }
>>     }
>>     if (local_err) {
>>         if (verbose == 0) {
>>             error_free(local_err);
>>         } else {
>>             error_report_err(local_err);
>>         }
>>     }
>> 
>> out:
>>     qio_channel_detach_aio_context(QIO_CHANNEL(client->ioc));
>>     object_unref(OBJECT(client->ioc));
>>     g_free(client);
>> }
>> 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]