Re: [libvirt] [PATCH v2 1/3] networkStartNetwork: Be more verbose
On 02/05/2014 12:11 PM, Michal Privoznik wrote:
The lack of debug printings might be frustrating in the future.
Moreover, this function doesn't follow the usual pattern we have in the
rest of the code:
int ret = -1;
/* do some work */
ret = 0;
cleanup:
/* some cleanup work */
return ret;
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/network/bridge_driver.c | 35 +++++++++++++++++++----------------
1 file changed, 19 insertions(+), 16 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index c8b167b..2bd015b 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1992,23 +1992,29 @@ static int
networkStartNetwork(virNetworkDriverStatePtr driver,
virNetworkObjPtr network)
{
- int ret = 0;
+ int ret = -1;
+
+ VIR_DEBUG("driver=%p, network=%p", driver, network);
if (virNetworkObjIsActive(network)) {
virReportError(VIR_ERR_OPERATION_INVALID,
"%s", _("network is already active"));
- return -1;
+ return ret;
Okay, this fixes the one problem I saw with v1 (calling
UnsetDefTransient on cleanup for a network that was already active).
}
+ VIR_DEBUG("Beginning network startup process");
+
+ VIR_DEBUG("Setting current network def as transient");
if (virNetworkObjSetDefTransient(network, true) < 0)
- return -1;
+ goto cleanup;
I *think* it's safe to call UnsetDefTransient when SetDefTransient
failed, so this is probably okay. And the rest of cleanup (which is
mostly just a call to networkShutdownNetwork(), after saving any errors)
is safe as well - it's a NOP if the network isn't active.
ACK.
switch (network->def->forward.type) {
case VIR_NETWORK_FORWARD_NONE:
case VIR_NETWORK_FORWARD_NAT:
case VIR_NETWORK_FORWARD_ROUTE:
- ret = networkStartNetworkVirtual(driver, network);
+ if (networkStartNetworkVirtual(driver, network) < 0)
+ goto cleanup;
break;
case VIR_NETWORK_FORWARD_BRIDGE:
@@ -2016,28 +2022,25 @@ networkStartNetwork(virNetworkDriverStatePtr driver,
case VIR_NETWORK_FORWARD_VEPA:
case VIR_NETWORK_FORWARD_PASSTHROUGH:
case VIR_NETWORK_FORWARD_HOSTDEV:
- ret = networkStartNetworkExternal(driver, network);
+ if (networkStartNetworkExternal(driver, network) < 0)
+ goto cleanup;
break;
}
- if (ret < 0) {
- virNetworkObjUnsetDefTransient(network);
- return ret;
- }
-
/* Persist the live configuration now that anything autogenerated
* is setup.
*/
- if ((ret = virNetworkSaveStatus(driverState->stateDir,
- network)) < 0) {
- goto error;
- }
+ VIR_DEBUG("Writing network status to disk");
+ if (virNetworkSaveStatus(driverState->stateDir, network) < 0)
+ goto cleanup;
- VIR_INFO("Starting up network '%s'", network->def->name);
network->active = 1;
+ VIR_INFO("Network '%s' started up", network->def->name);
+ ret = 0;
-error:
+cleanup:
if (ret < 0) {
+ virNetworkObjUnsetDefTransient(network);
virErrorPtr save_err = virSaveLastError();
int save_errno = errno;
networkShutdownNetwork(driver, network);