On 2/19/19 8:27 AM, Andrea Bolognani wrote:
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@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.
My recollection is/was event based mechanism and the HandleDumpCompleted was what ended up being where the message came through whether success or failure. Could be wrong - I perhaps had more in short term recall when I added some processing for memory dump event processing.
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?
Sure that's fine. I understand it doesn't mean someone will look at the commit message, but for those that do it can help.
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 :)
Thankfully gitk makes it easy for me to trace through the history. I know everyone has their favorite blame tools. John