On Thu, Dec 14, 2023 at 10:17:01 +0100, Michal Prívozník wrote:
> On 12/13/23 12:04, John Levon wrote:
> >
> > These two functions are currently private to libvirt, hence not available to
> > consumers. Would it be possible to make them public? Without them, there's no
> > way to do any libvirt call without stomping on an existing error that you may
> > want to preserve.
> >
> > I have multiple threads sharing a remote connection (to local libvirtd). I need
> > to be able to handle connection drops (e.g. libvirtd restart). Long story short,
> > the only approach I've found that actually works properly is that there's one
> > main conn object, each thread has a virConnectRef() to it, and whenever a thread
> > gets an error, we check in the error callback if !virConnectIsAlive(). If so, we
> > close the thread's conn, and potentially also clean up the main conn.
> >
> > Then when the error gets returned from the relevant library call, we try to
> > re-connect.
> >
> > However, virConnectIsAlive() stomps on the thread's error, meaning we lose the
> > context on a real, non-connection-related error.
> >
> > So I need to be able to save and restore in the error handler.
> >
> > Given how much these are used inside libvirt code already, it feels likely it
> > would be useful in other public cases too.
> >
> > Thoughts?
> >
>
> Yeah, it makes sense. I can see it used also in cleanup code (just like
> we do in our code). For instance:
>
>
> void func()
> {
>
> ...
> if (virDomainCreate(dom) < 0)
> goto cleanup;
> ...
> cleanup:
> /* save error here */
> if (dom)
> virDomainDestroy(dom);
> if (conn)
> virConnectClose(conn);
> ...
> /* restore error here */
> }
>
> Trouble is that every public API resets the error (by design). And while
> the error struct is public (which means users can write functions to
> create its copy), we already do have functions for that. So let's just
> expose them.
While I have no problems with virErrorPreserveLast() as that's just
syntax sugar on top of existing public functions, I don't think that
it's a good idea to expose virErrorRestore() as that gives the access to
previously unexposed virSetError(). This gives the users possibility to
set arbitrary errors due to the error struct being public as noted
above.
Saving an error to a custom error object should be enough and I don't
really see a point in making libvirt carry it around with the
possibility of it being overwritten at any point by another library
call.
Apps can then pass the object instance around as needed with more
explicit logic behind it.
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
