Re: [PATCH] network: bridge_driver: Use new helpers for storing libvirt errors
On 2/24/20 1:58 PM, Gaurav Agrawal wrote:
From: GAURAV AGRAWAL <agrawalgaurav(a)gnome.org>
Signed-off-by: Gaurav Agrawal <agrawalgaurav(a)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_virError...
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)