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").