[libvirt] [PATCH v4 2/7] parallels: fix parallelsLoadNetworks
Michal Privoznik
mprivozn at redhat.com
Wed Mar 18 17:34:23 UTC 2015
On 18.03.2015 09:33, Maxim Nestratov wrote:
> Don't fail initialization of parallels driver if
> parallelsLoadNetwork fails for optional networks.
> This can happen when some of them are added manually
> and configured incompletely. PCS requires only two networks
> created automatically (named Host-Only and Bridged), others
> are optional and their incompletenes can be ignored.
s/incompletenes/incompleteness/
>
> Signed-off-by: Maxim Nestratov <mnestratov at parallels.com>
> ---
> src/parallels/parallels_network.c | 43 +++++++++++++++++++++++-------------
> 1 files changed, 27 insertions(+), 16 deletions(-)
>
> diff --git a/src/parallels/parallels_network.c b/src/parallels/parallels_network.c
> index bb7ec5e..1d0ee1c 100644
> --- a/src/parallels/parallels_network.c
> +++ b/src/parallels/parallels_network.c
> @@ -180,9 +180,10 @@ static int parallelsGetHostOnlyNetInfo(virNetworkDefPtr def, const char *name)
> return ret;
> }
>
> -static virNetworkObjPtr
> +static int
> parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr jobj)
> {
> + int ret = -1;
> virNetworkObjPtr net;
> virNetworkDefPtr def;
> const char *tmp;
> @@ -214,13 +215,25 @@ parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr jobj)
> if (STREQ(tmp, PARALLELS_BRIDGED_NETWORK_TYPE)) {
> def->forward.type = VIR_NETWORK_FORWARD_BRIDGE;
>
> - if (parallelsGetBridgedNetInfo(def, jobj) < 0)
> + if (parallelsGetBridgedNetInfo(def, jobj) < 0) {
> +
> + /* Only mandatory networks are required to be configured completely */
> + if (STRNEQ(def->name, PARALLELS_REQUIRED_BRIDGED_NETWORK))
> + ret = 0;
> +
> goto cleanup;
> + }
> } else if (STREQ(tmp, PARALLELS_HOSTONLY_NETWORK_TYPE)) {
> def->forward.type = VIR_NETWORK_FORWARD_NONE;
>
> - if (parallelsGetHostOnlyNetInfo(def, def->name) < 0)
> + if (parallelsGetHostOnlyNetInfo(def, def->name) < 0) {
> +
> + /* Only mandatory networks are required to be configured completely */
> + if (STRNEQ(def->name, PARALLELS_REQUIRED_HOSTONLY_NETWORK))
> + ret = 0;
> +
> goto cleanup;
> + }
> } else {
> parallelsParseError();
> goto cleanup;
> @@ -230,14 +243,16 @@ parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr jobj)
> goto cleanup;
> net->active = 1;
> net->autostart = 1;
> - return net;
> + virNetworkObjEndAPI(&net);
> + ret = 0;
> + def = NULL;
So if everything goes well, zero is returned ...
>
> cleanup:
> virNetworkDefFree(def);
> - return NULL;
> + return ret;
> }
>
> -static virNetworkObjPtr
> +static int
> parallelsAddRoutedNetwork(parallelsConnPtr privconn)
> {
> virNetworkObjPtr net = NULL;
> @@ -264,18 +279,18 @@ parallelsAddRoutedNetwork(parallelsConnPtr privconn)
> }
> net->active = 1;
> net->autostart = 1;
> + virNetworkObjEndAPI(&net);
>
> - return net;
> + return 0;
>
> cleanup:
> virNetworkDefFree(def);
OUCH! Not to be seen in this context, but if virNetworkAssignDef() called a few lines above fails, it'll call virNetworkDefFree() and goto 'cleanup' subsequently where @def is freed the second time.
> - return NULL;
> + return -1;
> }
>
> static int parallelsLoadNetworks(parallelsConnPtr privconn)
> {
> virJSONValuePtr jobj, jobj2;
> - virNetworkObjPtr net = NULL;
> int ret = -1;
> int count;
> size_t i;
> @@ -300,22 +315,18 @@ static int parallelsLoadNetworks(parallelsConnPtr privconn)
> goto cleanup;
> }
>
> - net = parallelsLoadNetwork(privconn, jobj2);
> - if (!net)
> + ret = parallelsLoadNetwork(privconn, jobj2);
> + if (!ret)
... so this should have been (ret) instead of (!ret).
> goto cleanup;
> - else
> - virNetworkObjEndAPI(&net);
> -
> }
>
> - if (!(net = parallelsAddRoutedNetwork(privconn)))
> + if (!(ret = parallelsAddRoutedNetwork(privconn)))
Again, parallelsAddRoutedNetwork() returns zero on success.
> goto cleanup;
>
> ret = 0;
>
> cleanup:
> virJSONValueFree(jobj);
> - virNetworkObjEndAPI(&net);
> return ret;
> }
>
>
I'm squashing this in:
diff --git a/src/parallels/parallels_network.c b/src/parallels/parallels_network.c
index 1d0ee1c..8cc0582 100644
--- a/src/parallels/parallels_network.c
+++ b/src/parallels/parallels_network.c
@@ -184,7 +184,7 @@ static int
parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr jobj)
{
int ret = -1;
- virNetworkObjPtr net;
+ virNetworkObjPtr net = NULL;
virNetworkDefPtr def;
const char *tmp;
/* MD5_DIGEST_SIZE = VIR_UUID_BUFLEN = 16 */
@@ -241,13 +241,13 @@ parallelsLoadNetwork(parallelsConnPtr privconn, virJSONValuePtr jobj)
if (!(net = virNetworkAssignDef(privconn->networks, def, false)))
goto cleanup;
+ def = NULL;
net->active = 1;
net->autostart = 1;
- virNetworkObjEndAPI(&net);
ret = 0;
- def = NULL;
cleanup:
+ virNetworkObjEndAPI(&net);
virNetworkDefFree(def);
return ret;
}
@@ -273,10 +273,9 @@ parallelsAddRoutedNetwork(parallelsConnPtr privconn)
}
def->uuid_specified = 1;
- if (!(net = virNetworkAssignDef(privconn->networks, def, false))) {
- virNetworkDefFree(def);
+ if (!(net = virNetworkAssignDef(privconn->networks, def, false)))
goto cleanup;
- }
+
net->active = 1;
net->autostart = 1;
virNetworkObjEndAPI(&net);
@@ -315,12 +314,11 @@ static int parallelsLoadNetworks(parallelsConnPtr privconn)
goto cleanup;
}
- ret = parallelsLoadNetwork(privconn, jobj2);
- if (!ret)
+ if (parallelsLoadNetwork(privconn, jobj2) < 0)
goto cleanup;
}
- if (!(ret = parallelsAddRoutedNetwork(privconn)))
+ if (parallelsAddRoutedNetwork(privconn) < 0)
goto cleanup;
ret = 0;
Michal
More information about the libvir-list
mailing list