[libvirt] [PATCH V2 1/2] libxl: support interface type=network

[Laine Stump]

On 06/11/2014 12:25 AM, Jim Fehlig wrote:
> Add support for <interface type='network'> in the libxl driver.
>
> Signed-off-by: Jim Fehlig <jfehlig at suse.com>
> ---
>
> V2:
> Support both libvirt's traditional managed networks and libvirt
> networks that use an existing host bridge.
>
>  src/libxl/libxl_conf.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 48 insertions(+), 3 deletions(-)
>
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index cec37d6..9c453d8 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -874,6 +874,7 @@ libxlMakeNic(virDomainDefPtr def,
>               libxl_device_nic *x_nic)
>  {
>      bool ioemu_nic = STREQ(def->os.type, "hvm");
> +    virDomainNetType actual_type = virDomainNetGetActualType(l_nic);
>  
>      /* TODO: Where is mtu stored?
>       *
> @@ -899,16 +900,60 @@ libxlMakeNic(virDomainDefPtr def,
>      if (VIR_STRDUP(x_nic->ifname, l_nic->ifname) < 0)
>          return -1;
>  
> -    switch (l_nic->type) {
> +    switch (actual_type) {
>          case VIR_DOMAIN_NET_TYPE_BRIDGE:
> -            if (VIR_STRDUP(x_nic->bridge, l_nic->data.bridge.brname) < 0)
> +            if (VIR_STRDUP(x_nic->bridge,
> +                           virDomainNetGetActualBridgeName(l_nic)) < 0)
>                  return -1;
>              /* fallthrough */
>          case VIR_DOMAIN_NET_TYPE_ETHERNET:
>              if (VIR_STRDUP(x_nic->script, l_nic->script) < 0)
>                  return -1;
>              break;
> -        default:
> +        case VIR_DOMAIN_NET_TYPE_NETWORK:
> +        {
> +            bool fail = false;
> +            char *brname = NULL;
> +            virNetworkPtr network;
> +            virConnectPtr conn;
> +            virErrorPtr errobj;
> +
> +            if (!(conn = virConnectOpen("xen:///system")))
> +                return -1;

(The conn is necessary in order to call the functions that retrieve the
bridge device name for the network)

One difference in the way that qemu deals with this is that in most
cases the higher level function already has a conn ptr, and that is
passed down the call stack to the user. There is one case where this
doesn't happen - the autostart of a domain. In that case, qemu calls
virConnectOpen(cfg->uri) to get a conn.

In this case, you are always creating a new conn, even when one may
already exist a few layers up the call stack, and the conn you create is
using the hardcoded "xen:///system" rather than taking something from
config.

As long as this is okay with you, it's okay with me. I just wonder about
two things:

1) does xen support non-privileged libvirt, and if so what would happen
in this case (I suppose it would just fail due to lack of authorization).

2) is the uri configurable anywhere, as it is for libvirt?


> +
> +            if (!(network =
> +                  virNetworkLookupByName(conn, l_nic->data.network.name))) {
> +                virObjectUnref(conn);
> +                return -1;
> +            }
> +
> +            if ((brname = virNetworkGetBridgeName(network))) {
> +                if (VIR_STRDUP(x_nic->bridge, brname) < 0)
> +                    fail = true;
> +            } else {
> +                fail = true;
> +            }
> +
> +            VIR_FREE(brname);
> +
> +            /* Preserve any previous failure */
> +            errobj = virSaveLastError();
> +            virNetworkFree(network);
> +            virSetError(errobj);
> +            virFreeError(errobj);
> +            virObjectUnref(conn);
> +            if (fail)
> +                return -1;
> +            break;
> +        }
> +        case VIR_DOMAIN_NET_TYPE_USER:
> +        case VIR_DOMAIN_NET_TYPE_SERVER:
> +        case VIR_DOMAIN_NET_TYPE_CLIENT:
> +        case VIR_DOMAIN_NET_TYPE_MCAST:
> +        case VIR_DOMAIN_NET_TYPE_INTERNAL:
> +        case VIR_DOMAIN_NET_TYPE_DIRECT:
> +        case VIR_DOMAIN_NET_TYPE_HOSTDEV:
> +        case VIR_DOMAIN_NET_TYPE_LAST:
>              virReportError(VIR_ERR_INTERNAL_ERROR,
>                      _("libxenlight does not support network device type %s"),
>                      virDomainNetTypeToString(l_nic->type));

ACK, pending the answers to the above questions.