[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/3] xenconfig: add support for openvswitch configuration



On 11/16/18 11:26 PM, Jim Fehlig wrote:
> Add support for converting openvswitch interface configuration
> to/from libvirt domXML and xl.cfg(5). The xl config syntax for
> virtual interfaces is described in detail in the
> xl-network-configuration(5) man page. The Xen Networking wiki
> also contains information and examples for using openvswitch
> in xl.cfg config format
> 
> https://wiki.xenproject.org/wiki/Xen_Networking#Open_vSwitch
> 
> Tests are added to check conversions of openvswitch tagged and
> trunked VLAN configuration.
> 
> Signed-off-by: Jim Fehlig <jfehlig suse com>
> ---
>  src/xenconfig/xen_common.c                    | 113 +++++++++++++++++-
>  .../test-fullvirt-ovswitch-tagged.cfg         |  25 ++++
>  .../test-fullvirt-ovswitch-tagged.xml         |  50 ++++++++
>  .../test-fullvirt-ovswitch-trunked.cfg        |  25 ++++
>  .../test-fullvirt-ovswitch-trunked.xml        |  51 ++++++++
>  tests/xlconfigtest.c                          |   2 +
>  6 files changed, 262 insertions(+), 4 deletions(-)
> 
> diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c
> index 0a9958711f..5390b933e0 100644
> --- a/src/xenconfig/xen_common.c
> +++ b/src/xenconfig/xen_common.c
> @@ -856,6 +856,84 @@ xenParseCharDev(virConfPtr conf, virDomainDefPtr def, const char *nativeFormat)
>  }
>  
>  
> +static int
> +xenParseVifBridge(virDomainNetDefPtr net, char *bridge)
> +{
> +    char *vlanstr;
> +    unsigned int tag;
> +
> +    /* 'bridge' string contains a bridge name and single vlan tag */
> +    vlanstr = strchr(bridge, '.');
> +    if (vlanstr) {
> +        if (VIR_STRNDUP(net->data.bridge.brname, bridge, vlanstr - bridge) < 0)
> +            return -1;
> +
> +        vlanstr++;
> +        if (virStrToLong_ui(vlanstr, NULL, 10, &tag) < 0)
> +            return -1;
> +
> +        if (VIR_ALLOC_N(net->vlan.tag, 1) < 0)
> +            return -1;
> +
> +        net->vlan.tag[0] = tag;
> +        net->vlan.nTags = 1;
> +
> +        if (VIR_ALLOC(net->virtPortProfile) < 0)
> +            return -1;
> +
> +        net->virtPortProfile->virtPortType = VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH;
> +        return 0;
> +    }
> +
> +    /* 'bridge' string contains a bridge name and one or more vlan trunks */
> +    vlanstr = strchr(bridge, ':');
> +    if (vlanstr) {
> +        size_t i;
> +        size_t nvlans = 0;
> +        char **vlanstr_list = virStringSplit(bridge, ":", 0);
> +
> +        if (!vlanstr_list)
> +            return -1;
> +
> +        if (VIR_STRDUP(net->data.bridge.brname, vlanstr_list[0]) < 0) {
> +            virStringListFree(vlanstr_list);
> +            return -1;
> +        }
> +
> +        for (i = 1; vlanstr_list[i]; i++)
> +            nvlans++;
> +
> +        if (VIR_ALLOC_N(net->vlan.tag, nvlans) < 0) {
> +            virStringListFree(vlanstr_list);
> +            return -1;
> +        }
> +
> +        for (i = 1; i <= nvlans; i++) {
> +            if (virStrToLong_ui(vlanstr_list[i], NULL, 10, &tag) < 0) {
> +                virStringListFree(vlanstr_list);
> +                return -1;
> +            }
> +            net->vlan.tag[i - 1] = tag;
> +        }
> +        net->vlan.nTags = nvlans;
> +        net->vlan.trunk = true;
> +        virStringListFree(vlanstr_list);
> +
> +        if (VIR_ALLOC(net->virtPortProfile) < 0)
> +            return -1;
> +
> +        net->virtPortProfile->virtPortType = VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH;
> +        return 0;
> +    }
> +
> +    /* 'bridge' string only contains the bridge name */
> +    if (VIR_STRDUP(net->data.bridge.brname, bridge) < 0)
> +        return -1;
> +
> +    return 0;

Not a show stopper, I just wonder if we should perhaps use:

if ((vlanstr = strchr(bridge, '.'))) {
...
} else if ((vlanstr = strchr(bridge, ':'))) {
...
} else {
...
}

return 0;

To me that looks a bit cleaner, but I'll leave it up to you. Don't want
to be picky.

> +}
> +
> +
>  static virDomainNetDefPtr
>  xenParseVif(char *entry, const char *vif_typename)
>  {
> @@ -974,8 +1052,8 @@ xenParseVif(char *entry, const char *vif_typename)
>          net->type = VIR_DOMAIN_NET_TYPE_ETHERNET;
>      }
>  
> -    if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE) {
> -        if (bridge[0] && VIR_STRDUP(net->data.bridge.brname, bridge) < 0)
> +    if (net->type == VIR_DOMAIN_NET_TYPE_BRIDGE && bridge[0]) {
> +        if (xenParseVifBridge(net, bridge) < 0)
>              goto cleanup;
>      }
>      if (ip[0]) {

Michal


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]