Re: [PATCH] Revert "report error when virProcessGetStatInfo() is unable to parse data"

On Fri, 21 Jan 2022, Michal Prívozník wrote: > On 1/20/22 18:23, Ani Sinha wrote: > > > > > > On Thu, Jan 20, 2022 at 21:29 Michal Prívozník <mprivozn(a)redhat.com > > <mailto:mprivozn@redhat.com>> wrote: > > > > On 1/20/22 16:48, Ani Sinha wrote: > > > > > > > > > > > > > > AKA kicking the can one more time 🙃 > > > > Well, I should have been more careful and not merge the patch in the > > first place. Changing API behavior is something we should never do. > > > > Looking at the code closer, it looks like all callers of this function > > would need to ignore the reported error so that their behavior is not > > changed. At this point, does it make sense to report an error in the > > function? > > > > > > The callers can decide what do with the error raised by the function. We > > should not write functions that cannot fail. > > > > But that's not what the commit does, is it. It changed some public APIs > from best effort to fail early. That is the side effect and I consider it as a bug. Imagine we add some more code into that function tomorrow and there is an error path. Now because of this bug, we will have to ignore the error condition and not report it. How ridiculous is that? We should have kept the patch as is and fix the callers so that the public APIs were not broken.