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

[Ani Sinha]

On Fri, Jan 21, 2022 at 20:33 Daniel P. Berrangé <berrange at redhat.com>
wrote:

> On Fri, Jan 21, 2022 at 08:16:01PM +0530, Ani Sinha wrote:
> >
> >
> > On Fri, 21 Jan 2022, Daniel P. Berrangé wrote:
> >
> > > On Fri, Jan 21, 2022 at 07:38:35PM +0530, Ani Sinha wrote:
> > > >
> > > >
> > > > 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 at redhat.com
> > > > > > <mailto:mprivozn at 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.
> > >
> > > That is not the libvirt approach. Our #1 priority is public API
> > > compatibility, everything else is subservient to that. So when we
> > > have a regression we fix that in the quickest possible way. Simply
> > > reverting the broken patch>
> >
> > (a) I take exception to the fact that the patch reverted was "broken".
>
> Did you actually read the bug report in the first message in this
> series ? An end user hit a behaviour regression breaking the
> installation process, and was caused by the patch that's been
> reverted.


No I did not read the bug report because it required me to create an
account and then login. I do not have my redhat BZ account credentials
handy.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20220121/3bbd9b7e/attachment-0001.htm>