Re: [PATCH] libxl: support interface type=network

[Jim Fehlig <jfehlig@xxxxxxxx>]

Jim Fehlig wrote:
> Add support for <interface type='network'> in the libxl driver.
>
> Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
> ---
>  src/libxl/libxl_conf.c | 41 +++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 39 insertions(+), 2 deletions(-)
>
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index cec37d6..6efcea6 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -908,7 +908,44 @@ libxlMakeNic(virDomainDefPtr def,
>              if (VIR_STRDUP(x_nic->script, l_nic->script) < 0)
>                  return -1;
>              break;
> -        default:
> +        case VIR_DOMAIN_NET_TYPE_NETWORK:
> +        {
> +            bool error = true;
> +            char *brname = NULL;
> +            virNetworkPtr network = NULL;
> +            virConnectPtr conn;
> +
> +            if (!(conn = virConnectOpen("xen:///system")))
> +                return -1;
> +
> +            if (!(network =
> +                  virNetworkLookupByName(conn, l_nic->data.network.name)))
> +                goto cleanup_net;
> +
> +            if (!(brname = virNetworkGetBridgeName(network)))
> +                goto cleanup_net;
> +
> +            if (VIR_STRDUP(x_nic->bridge, brname) < 0)
> +                goto cleanup_net;
> +
> +            error = false;
> +
> +         cleanup_net:
> +            VIR_FREE(brname);
> +            virNetworkFree(network);
>   

While testing this patch a bit more, noticed that freeing the
virNetworkPtr here swallowed any previous errors.  E.g. when specifying
a non-existent network, I got the following error

error: invalid network pointer in virNetworkFree

With the below squashed in, I get the more reasonable

error: Network not found: no network with matching name 'foobar'

Regards,
Jim


diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 6efcea6..195bacc 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -910,31 +910,37 @@ libxlMakeNic(virDomainDefPtr def,
             break;
         case VIR_DOMAIN_NET_TYPE_NETWORK:
         {
-            bool error = true;
+            bool fail = false;
             char *brname = NULL;
-            virNetworkPtr network = NULL;
+            virNetworkPtr network;
             virConnectPtr conn;
+            virErrorPtr errobj;
 
             if (!(conn = virConnectOpen("xen:///system")))
                 return -1;
 
             if (!(network =
-                  virNetworkLookupByName(conn, l_nic->data.network.name)))
-                goto cleanup_net;
-
-            if (!(brname = virNetworkGetBridgeName(network)))
-                goto cleanup_net;
-
-            if (VIR_STRDUP(x_nic->bridge, brname) < 0)
-                goto cleanup_net;
+                  virNetworkLookupByName(conn,
l_nic->data.network.name))) {
+                virObjectUnref(conn);
+                return -1;
+            }
 
-            error = false;
+            if ((brname = virNetworkGetBridgeName(network))) {
+                if (VIR_STRDUP(x_nic->bridge, brname) < 0)
+                    fail = true;
+            } else {
+                fail = true;
+            }
 
-         cleanup_net:
             VIR_FREE(brname);
+
+            /* Preserve any previous failure */
+            errobj = virSaveLastError();
             virNetworkFree(network);
+            virSetError(errobj);
+            virFreeError(errobj);
             virObjectUnref(conn);
-            if (error)
+            if (fail)
                 return -1;
             break;
         }

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list