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@xxxxxxxxxx> >>> --- >>> 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
