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

[Ani Sinha]

On Fri, Jan 21, 2022 at 8:56 PM Daniel P. Berrangé <berrange at redhat.com>
wrote:

> On Fri, Jan 21, 2022 at 08:39:47PM +0530, Ani Sinha wrote:
> > 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.
>
> Urgh, sorry I missed that the BZ was private. Private BZs are not
> supposed to be listed in commit messages for precisely the reason
> that people can't see them. The commit message should have showed
> the problem listed in the private BZ description instead.


Turns out even if I logged into BZ with my old account credentials ( I
couldn't find the password, so reset it) I still can't access the bug info:

You are not authorized to access bug #2041610.

Most likely the bug has been restricted for internal development processes
and we cannot grant access.

If you are a Red Hat customer with an active subscription, please visit the Red
Hat Customer Portal <https://access.redhat.com/> for assistance with your
issue

If you are a Fedora Project user and require assistance, please consider
using one of the mailing lists <https://fedoraproject.org/wiki/Communicate> we
host for the Fedora Project.


>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20220121/7e5e21ce/attachment-0001.htm>