[libvirt] [PATCH v2 3/3] iohelper: Don't include newlines in error messages

[John Ferlan]

On 2/19/19 8:44 AM, Andrea Bolognani wrote:
> On Mon, 2019-02-18 at 11:05 -0500, John Ferlan wrote:
>> On 2/13/19 7:04 AM, Andrea Bolognani wrote:
>>> The newline was pretty arbitrary, and we're better off
>>> without it.
>>
>> I'm mostly ambivalent about this one; however, since
>> virGetLastErrorMessage could return a string without a "\n", then
>> perhaps it's best to keep the \n since it really doesn't hurt.
> 
> Without this patch, the user will end up seeing
> 
>   $ sudo virsh dump guest /small/guest.dump --memory-only
>   error: Failed to core dump domain guest to /small/guest.dump
>   error: operation failed: /usr/libexec/libvirt_iohelper: failure with /small/guest.dump
>   : Unable to write /small/guest.dump: No space left on device
> 
> instead of the more reasonable
> 
>   $ sudo virsh dump guest /small/guest.dump --memory-only
>   error: Failed to core dump domain guest to /small/guest.dump
>   error: operation failed: /usr/libexec/libvirt_iohelper: failure with /small/guest.dump: Unable to write /small/guest.dump: No space left on device
> 
> Now, neither is optimal and the way libvirt_iohelper formats its
> error messages should be tweaked further, but at least in the latter
> case the error is not split randomly with the second line starting
> with a colon, which is an improvement in my book.
> 
> I was also unable to find other examples of messages passed to
> virReportError(), which is what will ultimately happen to this
> output, containing newlines. To be fair, I have not really spent a
> lot of time looking for them either :)
> 

I didn't dig too much either, but I was perhaps overthinking the other
end of the message. I went straight to virGetLastErrorMessage and noted
it "could" return "no error" or "unknown error" without the '\n' at the
end which IIUC would be generated for virReportError (or err->message)
values.

Anyway, see commit b29e08db... I think a case could be made in the
commit message ;-) that prior to that commit the '\n' was (properly) at
the end, but with that commit message the '\n' was perhaps put in the
wrong place.

So maybe this changes to move the '\n' to after the formatted message.
Of course that means perhaps an extra blank line for most outputs.

In the long run, it's a blank line adjustment, so consider this

Reviewed-by: John Ferlan <jferlan at redhat.com>

for me too (I see Dan responded in the mean time).

John