<div dir="auto">Hi Laine,<div dir="auto"><br></div><div dir="auto">I am sure that I did --subject-prefix , not sure why it did not landed.</div><div dir="auto"><br></div><div dir="auto">Now am wondering about this situation do I still need a PATCH-3 or it's handled ?</div><div dir="auto"><br></div><div dir="auto">Thanks for giving your one last pass!</div><div dir="auto"><br></div><div dir="auto">Best Regards</div><div dir="auto">Gaurav</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Feb 25, 2020, 22:46 Laine Stump <<a href="mailto:laine@redhat.com">laine@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 2/25/20 7:37 AM, Ján Tomko wrote:<br>
> On Mon, Feb 24, 2020 at 10:09:05PM -0500, Laine Stump wrote:<br>
>> On 2/24/20 1:58 PM, Gaurav Agrawal wrote:<br>
>><br>
><br>
> Yes, networkSetupPrivateChains is only called once (via virOnce, as<br>
> suggested by the comment on the top of the function) on initialization<br>
> and if either IPv4 or IPv6 chains could not be created, it sets the<br>
> dirver-global error, which is then called on any subsequent attempt<br>
> to use it.<br>
><br>
> So this is not really a case that needs to be converted.<br>
> In fact, glancing at git gre virSetError it seems we already got rid<br>
> of all the ones worth converting.<br>
<br>
<br>
I could have sworn that when I looked last night there were a handful of <br>
virSaveLastError() calls, and that I looked at one that was paired with <br>
virSetError(). But when I look now I see that all the virSaveLastError() <br>
calls remaining are strange "save this for later" type things rather <br>
than "save this for a second while we clean up the mess".<br>
<br>
<br>
It looks like jferlan pushed a bunch of patches in Dec 2018 to do all <br>
the valid replacements, but the item was never removed from the <br>
bite-sized tasks list (he might have fixed them without even knowing <br>
about the item on the list). I just went to the wiki to remove it and <br>
see that Jano has already taken care of it!<br>
<br>
<br>
><br>
><br>
>><br>
>> Your patch has replaced the virSaveLastError() of the earlier part <br>
>> with virErrorPreserveLast(), but hasn't replaced the virSetError() of <br>
>> the later part (which is down in networkAddFirewallRules()) with <br>
>> virErrorRestore().<br>
><br>
> virErrorRestore resets the error, which is not what we want here -<br>
> any subsequent calls should report the same error we caught when<br>
> initializing.<br>
<br>
<br>
Yeah, I realized that was probably the case in the middle of the night <br>
last night, but wasn't at the keyboard to chastise myself. I knew I <br>
should have just gone to bed instead of sitting down for one last pass...<br>
<br>
<br>
But anyway the upside is that Guarav got git send-email configured <br>
properly to send future patches (while we're on the topic of workflow - <br>
when you send a modified/updated version of a patch, be sure to note <br>
that in the subject, e.g. with "--subject-prefix="PATCHv2").<br>
<br>
<br>
<br>
</blockquote></div>