[PATCH] network: bridge_driver: Use new helpers for storing libvirt errors

[Gaurav Agrawal]

Hi Laine,

I am sure that I did --subject-prefix , not sure why it did not landed.

Now am wondering about this situation do I still need a PATCH-3 or it's
handled ?

Thanks for giving your one last pass!

Best Regards
Gaurav

On Tue, Feb 25, 2020, 22:46 Laine Stump <laine at redhat.com> wrote:

> On 2/25/20 7:37 AM, Ján Tomko wrote:
> > On Mon, Feb 24, 2020 at 10:09:05PM -0500, Laine Stump wrote:
> >> On 2/24/20 1:58 PM, Gaurav Agrawal wrote:
> >>
> >
> > Yes, networkSetupPrivateChains is only called once (via virOnce, as
> > suggested by the comment on the top of the function) on initialization
> > and if either IPv4 or IPv6 chains could not be created, it sets the
> > dirver-global error, which is then called on any subsequent attempt
> > to use it.
> >
> > So this is not really a case that needs to be converted.
> > In fact, glancing at git gre virSetError it seems we already got rid
> > of all the ones worth converting.
>
>
> I could have sworn that when I looked last night there were a handful of
> virSaveLastError() calls, and that I looked at one that was paired with
> virSetError(). But when I look now I see that all the virSaveLastError()
> calls remaining are strange "save this for later" type things rather
> than "save this for a second while we clean up the mess".
>
>
> It looks like jferlan pushed a bunch of patches in Dec 2018 to do all
> the valid replacements, but the item was never removed from the
> bite-sized tasks list (he might have fixed them without even knowing
> about the item on the list). I just went to the wiki to remove it and
> see that Jano has already taken care of it!
>
>
> >
> >
> >>
> >> Your patch has replaced the virSaveLastError() of the earlier part
> >> with virErrorPreserveLast(), but hasn't replaced the virSetError() of
> >> the later part (which is down in networkAddFirewallRules()) with
> >> virErrorRestore().
> >
> > virErrorRestore resets the error, which is not what we want here -
> > any subsequent calls should report the same error we caught when
> > initializing.
>
>
> Yeah, I realized that was probably the case in the middle of the night
> last night, but wasn't at the keyboard to chastise myself. I knew I
> should have just gone to bed instead of sitting down for one last pass...
>
>
> But anyway the upside is that Guarav got git send-email configured
> properly to send future patches (while we're on the topic of workflow -
> when you send a modified/updated version of a patch, be sure to note
> that in the subject, e.g. with "--subject-prefix="PATCHv2").
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200225/e41833f2/attachment-0001.htm>