Re: Public virErrorPreserveLast()/virErrorRestore()

On Thu, Dec 14, 2023 at 12:03:55 +0000, John Levon wrote: Please note that virErrorRestore() is used (almost [1]) exclusively inside libvirt's library code, thus *before* the error itself gets raised to the caller of a public libvirt API. Also TBH I'm not a fan of the passing of errors via the thread local variable because, while it's convenient in the simple cases, for any more complicated cases it just creates headaches (even internally). As in your original problem statement it also makes problems when you want to associate the error itself with the function call that caused it. If you need any form of more complex logic in your application for error reporting (ab)using libvirt's thread local variable to store the error for other functions in your application will not bring too much simplification of the logic itself. Additionally care would still have to be taken to use e.g. libvirt's cleanup functions which would still reset the thread local error. Applications which wish to do delayed error reporting have the means to do so by having their own instance of virError, either per process (as virsh does), or any other granularity, including a thread local one if they wish to do so. To use such error reporting you can use virSaveLastError(), which retrieves the proper error object and then pass it via your own structures with known lifetimes and without the possibly surprising resetting of the error. Using the global error handler function is not a very good idea in multithreaded applications, so I'd really suggest to have a generic handler function that you'll call after receiving a failure code from any of the libvirt APIs you care about and that will preserve/raise the error in your application. Additionally for handling disconnects libvirt provides a connection close callback registrable via virConnectRegisterCloseCallback(), which can be registered multiple times with private data for each instance. That might help you getting rid of the generic error handler's duty to catch disconnects. I understand that the error reporting situation is not ideal but I'm still very sceptical that having the ability to reset the error would help much. [1] There is one use in virsh and one use in virt-admin code, which I'll be addressing.