[libvirt] [PATCH v2 2/3] virfile: Report error in virFileWrapperFdFree()

[Andrea Bolognani]

On Mon, 2019-02-18 at 11:04 -0500, John Ferlan wrote:
> 
> On 2/13/19 7:04 AM, Andrea Bolognani wrote:
> > Logging the error is fine and all, but getting the information
> > to the user directly is even better.
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=1578741
> > Signed-off-by: Andrea Bolognani <abologna at redhat.com>
> > ---
> >  src/util/virfile.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> 
> Shall I assume this works because qemuProcessHandleDumpCompleted will
> move this message now as opposed to some other message related to EPIPE
> as noted in the bz response?

libvirt_iohelper is used internally by the virFileWrapperFd APIs;
more specifically, in the QEMU driver we have the doCoreDump() and
qemuDomainSaveMemory() helper functions as users, and those in turn
end up being called by the implementation of several driver APIs.

By calling virReportError() if libvirt_iohelper has failed, we
overwrite whatever generic error message QEMU might have raised
with the more useful one generated by the helper program.

I'm not sure how qemuProcessHandleDumpCompleted() fits into the
picture.

> I think it's good to fill in pieces of the commit message at least so if
> someone else ends up in this same code they have a few hints where to start.

Yeah, I guess the commit message is a bit terse. Would adding
something along the lines of the first two paragraphs above work,
in your opinion?

> My other throughts would be to note commit 1f25194ad and 34e8f63a3 as
> the genesis of at least printing the message "somewhere"... Then commit
> b0c3e931 at least made sure the message got printed in the event that
> the *FdClose never occurred. Just saying "is fine and all" hand waves a
> bit too much for me compared to what you got to figure out here ;-).

Honestly, I didn't bother digging into the history of the
functionality and simply fixed what was broken without really
wondering how it ended up like that in the first place :)

-- 
Andrea Bolognani / Red Hat / Virtualization