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

Laine Stump laine at redhat.com
Tue Feb 25 03:09:05 UTC 2020


On 2/24/20 1:58 PM, Gaurav Agrawal wrote:
> From: GAURAV AGRAWAL <agrawalgaurav at gnome.org>
>
> Signed-off-by: Gaurav Agrawal <agrawalgaurav at gnome.org>
> ---
>   src/network/bridge_driver_linux.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)


Yay! It applied properly, so you have all the pieces of patch submission 
properly in line! :-)


(Welcome to the neighborhood, BTW.)


>
> diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
> index 7bbde5c6a9..fde33b5d38 100644
> --- a/src/network/bridge_driver_linux.c
> +++ b/src/network/bridge_driver_linux.c
> @@ -22,6 +22,7 @@
>   #include <config.h>
>   
>   #include "viralloc.h"
> +#include "virerror.h"
>   #include "virfile.h"
>   #include "viriptables.h"
>   #include "virstring.h"
> @@ -53,7 +54,7 @@ static void networkSetupPrivateChains(void)
>       if (rc < 0) {
>           VIR_DEBUG("Failed to create global IPv4 chains: %s",
>                     virGetLastErrorMessage());
> -        errInitV4 = virSaveLastError();
> +        virErrorPreserveLast(&errInitV4);
>           virResetLastError();
>       } else {
>           virFreeError(errInitV4);
> @@ -70,7 +71,7 @@ static void networkSetupPrivateChains(void)
>       if (rc < 0) {
>           VIR_DEBUG("Failed to create global IPv6 chains: %s",
>                     virGetLastErrorMessage());
> -        errInitV6 = virSaveLastError();
> +        virErrorPreserveLast(&errInitV6);
>           virResetLastError();
>       } else {
>           virFreeError(errInitV6);


For anyone who didn't notice, this patch is for one of the "bite sized 
tasks" here:

https://wiki.libvirt.org/page/BiteSizedTasks#More_conversions_to_virErrorPreserveLast.2FvirErrorRestore


This is a very strange case of virSaveLastError() / virSetError() to 
pick - usually they are in pairs within the same function, but in this 
case when the original error occurs during the driver init, it is 
squanched away in the static errInitV[46] with virSaveLastError(), and 
later, in a completely different context, whenever something tries to 
add a firewall rule of the given type, *then* it sets the error (with 
virSetError()) to what was earlier encountered during init.


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


I can make those two minor changes and push if you like, or you're free 
to send again with --subject-prefix="PATCHv2". (Or, maybe an even better 
idea - you could expand the patch to replace all the other uses of 
virSaveLastError()/virSetError()). The choice is yours :-)


(Oh, and BTW, no need to Cc: crobinso - he'll see it anyway)




More information about the libvir-list mailing list