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

Laine Stump laine at redhat.com
Tue Feb 25 17:16:42 UTC 2020


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






More information about the libvir-list mailing list