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