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

Re: [libvirt] [PATCH v7] vepa: parsing for 802.1Qb{g|h} XML



On Fri, 2010-05-21 at 16:46 -0700, Chris Wright wrote:
> * Stefan Berger (stefanb linux vnet ibm com) wrote:
> > Below is David Alan's original patch with lots of changes. 
> > 
> > In particular, it now parses the following two XML descriptions, one
> > for 802.1Qbg and 802.1Qbh and stored the data internally. The actual
> > triggering of the switch setup protocol has not been implemented
> > here but the relevant code to do that should go into the functions 
> > associatePortProfileId() and disassociatePortProfileId().
> 
> For future work, have you thought about preassociate on migration?

You mean first to do a preassociate and then after migration has
succeeded a (full) associate. I had thought about it. I believe the
current code would need to know what the actual cause was for it being
called. We currently handle it I believe correctly for

- 'direct' type of interface creation and destruction
- resume after a suspend

Now if we wanted to pre-associate on migration on the destination host,
we would need to know that the function was called due to a migration
operation in order not to call the associate function but rather the
pre-associate one. The I believe upon failure of migration the
disassociate would still be call-able on the target host, but in
addition we would need to call the associate function after the
migration was successful. We're not doing this. I suppose also that
nothing would be allowed to go wrong once one switches from
pre-associate to associate in such a scenario since the migration
succeeded and a failure to associate couldn't be handled gracefully
anymore (by failing migration).

[...]
> > 
> > Some of the changes from V1 to V2:
> > - parser and XML generator have been separated into their own
> >   functions so they can be re-used elsewhere (passthrough case
> >   for example)
> > - Adapted XML parser and generator support the above shown type
> >   (802.1Qbg, 802.1Qbh).
> > - Adapted schema to above XML
> > - Adapted test XML to above XML
> > - Passing through the VM's UUID which seems to be necessary for
> >   802.1Qbh -- sorry no host UUID
> > - adding virtual function ID to association function, in case it's
> >   necessary to use (for SR-IOV)
> > 
> > Signed-off-by: Stefan Berger <stefanb us ibm com>
> 
> This is looking like pretty complete infrastructure to me.  Have you
> been able generate test (dis)associate messages through this framework w/
> the vsi patches?  Pending that (I can see you are able to at least
> generate debug messages showing the thing work), and some minor nits
> below.

I have been able to communicate with a self-hacked dummy server for
802.1Qbg that receives a netlink message and responds with a netlink
message via unicast to libvirt. With that server I am currently sending
the response immediately to the RTM_SETLINK message, which I believe
isn't what the kernel is doing. Rather it should send it back upon
RTM_GETLINK. If that server is missing or indicates failure to start, I
am timing out and fail the start of the VM. The disassociate part is in
the path of the tear down code and is being invoked as well. I haven't
tested much with that for 802.1Qbg.

> 
> Acked-by: Chris Wright <chrisw redhat com>
> 
> > >From a945107f047c7cd71f9c1b74fd74c47d8cdc3670 Mon Sep 17 00:00:00 2001
> > From: David Allan <dallan redhat com>
> > Date: Fri, 12 Mar 2010 13:25:04 -0500
> > Subject: [PATCH 1/1] POC of port profile id support
> > 
> > * Modified schema per DanPB's feedback
> > * Added test for modified schema
> > ---
> >  docs/schemas/domain.rng                |   69 ++++++++++++++
> >  src/conf/domain_conf.c                 |  155 +++++++++++++++++++++++++++++++++
> >  src/conf/domain_conf.h                 |   35 +++++++
> >  src/qemu/qemu_conf.c                   |   18 +--
> >  src/qemu/qemu_conf.h                   |    5 -
> >  src/qemu/qemu_driver.c                 |   17 +--
> >  src/util/macvtap.c                     |  151 +++++++++++++++++++++++++++-----
> >  src/util/macvtap.h                     |   10 +-
> >  tests/domainschemadata/portprofile.xml |   36 +++++++
> >  9 files changed, 446 insertions(+), 50 deletions(-)
> >  create mode 100644 tests/domainschemadata/portprofile.xml
> > 

> > Index: libvirt-acl/src/conf/domain_conf.c
> > ===================================================================
> > --- libvirt-acl.orig/src/conf/domain_conf.c
> > +++ libvirt-acl/src/conf/domain_conf.c
> > @@ -242,6 +242,11 @@ VIR_ENUM_IMPL(virDomainNetdevMacvtap, VI
> >                "private",
> >                "bridge")
> >  
> > +VIR_ENUM_IMPL(virVirtualPort, VIR_VIRTUALPORT_TYPE_LAST,
> > +              "none",
> > +              "802.1Qbg",
> > +              "802.1Qbh")
> > +
> >  VIR_ENUM_IMPL(virDomainClockOffset, VIR_DOMAIN_CLOCK_OFFSET_LAST,
> >                "utc",
> >                "localtime",
> > @@ -1807,6 +1812,145 @@ cleanup:
> >  }
> >  
> >  
> > +static void
> 
> static int so we can capture an error?

Ok.

> 
> > +virVirtualPortProfileDefParseXML(xmlNodePtr node,
> > +                                virVirtualPortProfileDefPtr virtPort)
> > +{
> > +    char *virtPortType;
> > +    char *virtPortManagerID = NULL;
> > +    char *virtPortTypeID = NULL;
> > +    char *virtPortTypeIDVersion = NULL;
> > +    char *virtPortInstanceID = NULL;
> > +    char *virtPortProfileID = NULL;
> > +    xmlNodePtr cur = node->children;
> 
> int ret = -1;
> 
> > +
> > +    virtPortType = virXMLPropString(node, "type");
> > +    if (!virtPortType)
> > +        return;
> > +
> > +    while (cur != NULL) {
> > +        if (xmlStrEqual(cur->name, BAD_CAST "parameters")) {
> > +
> > +            virtPortManagerID = virXMLPropString(cur, "managerid");
> > +            virtPortTypeID = virXMLPropString(cur, "typeid");
> > +            virtPortTypeIDVersion = virXMLPropString(cur, "typeidversion");
> > +            virtPortInstanceID = virXMLPropString(cur, "instanceid");
> > +            virtPortProfileID = virXMLPropString(cur, "profileid");
> > +
> > +            break;
> 
> s/break/goto out/
> 
> > +        }
> > +
> > +        cur = cur->next;
> > +    }
> > +
> > +    virtPort->virtPortType = VIR_VIRTUALPORT_NONE;
> > +
> > +    switch (virVirtualPortTypeFromString(virtPortType)) {
> > +
> > +    case VIR_VIRTUALPORT_8021QBG:
> > +        if (virtPortManagerID     != NULL && virtPortTypeID     != NULL &&
> > +            virtPortTypeIDVersion != NULL) {
> > +            unsigned int val;
> > +
> > +            if ((virStrToLong_ui(virtPortManagerID, NULL, 10, &val) &&
> > +                 virStrToLong_ui(virtPortManagerID, NULL, 16, &val)   ) ||
> 
> Why not a baseless strtol (0 instead of 10 and 16)?  And I think this
> should give some virDomainReportError() so that we can figure out what's
> happening when the xml is incorrect.

Done.

> 
> Ditto for parsing the whole thing...
> 
> > +                val > 0xff)
> > +                break;
> 
> s/break/goto out/
>

Ok.

> > +
> > +            virtPort->u.virtPort8021Qbg.managerID = (uint8_t)val;
> > +
> > +            if ((virStrToLong_ui(virtPortTypeID, NULL, 10, &val) &&
> > +                 virStrToLong_ui(virtPortTypeID, NULL, 16, &val)   ) ||
> > +                val > 0xffffff)
> > +                break;
> > +
> > +            virtPort->u.virtPort8021Qbg.typeID = (uint32_t)val;
> > +
> > +            if ((virStrToLong_ui(virtPortTypeIDVersion, NULL, 10, &val) &&
> > +                 virStrToLong_ui(virtPortTypeIDVersion, NULL, 16, &val)   ) ||
> > +                val > 0xff)
> > +                break;
> > +
> > +            virtPort->u.virtPort8021Qbg.typeIDVersion = (uint8_t)val;
> > +
> > +            if (virtPortInstanceID != NULL) {
> > +                if (virUUIDParse(virtPortInstanceID, virtPort->u.virtPort8021Qbg.instanceID))
> > +                    break;
> > +            } else {
> > +                if (virUUIDGenerate(virtPort->u.virtPort8021Qbg.instanceID))
> > +                    break;
> > +            }
> > +
> > +            virtPort->virtPortType = VIR_VIRTUALPORT_8021QBG;
> > +        }
> > +    break;
> > +
> > +    case VIR_VIRTUALPORT_8021QBH:
> > +        if (virtPortProfileID != NULL) {
> > +            if (virStrcpyStatic(virtPort->u.virtPort8021Qbh.profileID,
> > +                                virtPortProfileID) != NULL)
> > +                virtPort->virtPortType = VIR_VIRTUALPORT_8021QBH;
> > +        }
> > +    break;
> > +
> > +
> > +    default:
> > +    case VIR_VIRTUALPORT_NONE:
> > +    case VIR_VIRTUALPORT_TYPE_LAST:
> > +    break;
> 
> error cases, no?  goto out

Ok. However, I'd still allow the omission of <virtualport/> so I can
start a macvtap without having to have a (future) switch.

> 
> > +    }
> > +
> 
>        ret = 0;
> 
> out:
> 
> > +    VIR_FREE(virtPortManagerID);
> > +    VIR_FREE(virtPortTypeID);
> > +    VIR_FREE(virtPortTypeIDVersion);
> > +    VIR_FREE(virtPortInstanceID);
> > +    VIR_FREE(virtPortProfileID);
> > +    VIR_FREE(virtPortType);
> 
>        return ret;

Ok.

> > +}
> > +
> > +
> > +static void
> > +virVirtualPortProfileFormat(virBufferPtr buf, virVirtualPortProfileDefPtr virtPort,
> > +                            const char *indent)
> > +{
> > +    char uuidstr[VIR_UUID_STRING_BUFLEN];
> > +
> > +    if (virtPort->virtPortType == VIR_VIRTUALPORT_NONE)
> > +        return;
> > +
> > +    virBufferVSprintf(buf, "%s<virtualport type='%s'>\n",
> > +                      indent, virVirtualPortTypeToString(virtPort->virtPortType));
> > +
> > +    switch (virtPort->virtPortType) {
> > +    case VIR_VIRTUALPORT_NONE:
> > +    case VIR_VIRTUALPORT_TYPE_LAST:
> > +        break;
> > +
> > +    case VIR_VIRTUALPORT_8021QBG:
> > +        virUUIDFormat(virtPort->u.virtPort8021Qbg.instanceID,
> > +                      uuidstr);
> > +        virBufferVSprintf(buf,
> > +                          "%s  <parameters managerid='%d' typeid='%d' "
> > +                          "typeidversion='%d' instanceid='%s'/>\n",
> > +                          indent,
> > +                          virtPort->u.virtPort8021Qbg.managerID,
> > +                          virtPort->u.virtPort8021Qbg.typeID,
> > +                          virtPort->u.virtPort8021Qbg.typeIDVersion,
> > +                          uuidstr);
> > +        break;
> > +
> > +    case VIR_VIRTUALPORT_8021QBH:
> > +        virBufferVSprintf(buf,
> > +                          "%s  <parameters profileid='%s'/>\n",
> > +                          indent,
> > +                          virtPort->u.virtPort8021Qbh.profileID);
> > +        break;
> > +    }
> > +
> > +    virBufferVSprintf(buf, "%s</virtualport>\n", indent);
> > +}
> > +
> > +
> >  /* Parse the XML definition for a network interface
> >   * @param node XML nodeset to parse for net definition
> >   * @return 0 on success, -1 on failure
> > @@ -1832,6 +1976,8 @@ virDomainNetDefParseXML(virCapsPtr caps,
> >      char *devaddr = NULL;
> >      char *mode = NULL;
> >      virNWFilterHashTablePtr filterparams = NULL;
> > +    virVirtualPortProfileDef virtPort;
> > +    bool virtPortParsed = false;
> >  
> >      if (VIR_ALLOC(def) < 0) {
> >          virReportOOMError();
> > @@ -1873,6 +2019,11 @@ virDomainNetDefParseXML(virCapsPtr caps,
> >                         xmlStrEqual(cur->name, BAD_CAST "source")) {
> >                  dev  = virXMLPropString(cur, "dev");
> >                  mode = virXMLPropString(cur, "mode");
> > +            } else if ((virtPortParsed == false) &&
> > +                       (def->type == VIR_DOMAIN_NET_TYPE_DIRECT) &&
> > +                       xmlStrEqual(cur->name, BAD_CAST "virtualport")) {
> > +                virVirtualPortProfileDefParseXML(cur, &virtPort);
> > +                virtPortParsed = true;
> >              } else if ((network == NULL) &&
> >                         ((def->type == VIR_DOMAIN_NET_TYPE_SERVER) ||
> >                          (def->type == VIR_DOMAIN_NET_TYPE_CLIENT) ||
> > @@ -2048,6 +2199,8 @@ virDomainNetDefParseXML(virCapsPtr caps,
> >          } else
> >              def->data.direct.mode = VIR_DOMAIN_NETDEV_MACVTAP_MODE_VEPA;
> >  
> > +        def->data.direct.virtPortProfile = virtPort;
> > +
> >          def->data.direct.linkdev = dev;
> >          dev = NULL;
> >  
> > @@ -5140,6 +5293,8 @@ virDomainNetDefFormat(virBufferPtr buf,
> >          virBufferVSprintf(buf, " mode='%s'",
> >                     virDomainNetdevMacvtapTypeToString(def->data.direct.mode));
> >          virBufferAddLit(buf, "/>\n");
> > +        virVirtualPortProfileFormat(buf, &def->data.direct.virtPortProfile,
> > +                            "      ");
> >          break;
> >  
> >      case VIR_DOMAIN_NET_TYPE_USER:
> > Index: libvirt-acl/src/conf/domain_conf.h
> > ===================================================================
> > --- libvirt-acl.orig/src/conf/domain_conf.h
> > +++ libvirt-acl/src/conf/domain_conf.h
> > @@ -259,6 +259,39 @@ enum virDomainNetdevMacvtapType {
> >  };
> >  
> >  
> > +enum virVirtualPortType {
> > +    VIR_VIRTUALPORT_NONE,
> > +    VIR_VIRTUALPORT_8021QBG,
> > +    VIR_VIRTUALPORT_8021QBH,
> > +
> > +    VIR_VIRTUALPORT_TYPE_LAST,
> > +};
> > +
> > +# ifdef IFLA_VF_PORT_PROFILE_MAX
> > +#  define LIBVIRT_IFLA_VF_PORT_PROFILE_MAX IFLA_VF_PORT_PROFILE_MAX
> > +# else
> > +#  define LIBVIRT_IFLA_VF_PORT_PROFILE_MAX 40
> > +# endif
> > +
> > +/* profile data for macvtap (VEPA) */
> > +typedef struct _virVirtualPortProfileDef virVirtualPortProfileDef;
> > +typedef virVirtualPortProfileDef *virVirtualPortProfileDefPtr;
> > +struct _virVirtualPortProfileDef {
> > +    enum virVirtualPortType   virtPortType;
> > +    union {
> > +        struct {
> > +            uint8_t       managerID;
> > +            uint32_t      typeID; // 24 bit valid
> > +            uint8_t       typeIDVersion;
> > +            unsigned char instanceID[VIR_UUID_BUFLEN];
> > +        } virtPort8021Qbg;
> > +        struct {
> > +            char          profileID[LIBVIRT_IFLA_VF_PORT_PROFILE_MAX];
> > +        } virtPort8021Qbh;
> > +    } u;
> > +};
> > +
> > +
> >  /* Stores the virtual network interface configuration */
> >  typedef struct _virDomainNetDef virDomainNetDef;
> >  typedef virDomainNetDef *virDomainNetDefPtr;
> > @@ -290,6 +323,7 @@ struct _virDomainNetDef {
> >          struct {
> >              char *linkdev;
> >              int mode;
> > +            virVirtualPortProfileDef virtPortProfile;
> >          } direct;
> >      } data;
> >      char *ifname;
> > @@ -1089,6 +1123,7 @@ VIR_ENUM_DECL(virDomainSeclabel)
> >  VIR_ENUM_DECL(virDomainClockOffset)
> >  
> >  VIR_ENUM_DECL(virDomainNetdevMacvtap)
> > +VIR_ENUM_DECL(virVirtualPort)
> >  
> >  VIR_ENUM_DECL(virDomainTimerName)
> >  VIR_ENUM_DECL(virDomainTimerTrack)
> > Index: libvirt-acl/src/util/macvtap.c
> > ===================================================================
> > --- libvirt-acl.orig/src/util/macvtap.c
> > +++ libvirt-acl/src/util/macvtap.c
> > @@ -43,6 +43,7 @@
> >  
> >  # include "util.h"
> >  # include "memory.h"
> > +# include "logging.h"
> >  # include "macvtap.h"
> >  # include "interface.h"
> >  # include "conf/domain_conf.h"
> > @@ -57,6 +58,16 @@
> >  # define MACVTAP_NAME_PREFIX	"macvtap"
> >  # define MACVTAP_NAME_PATTERN	"macvtap%d"
> >  
> > +
> > +static int associatePortProfileId(const char *macvtap_ifname,
> > +                                  const virVirtualPortProfileDefPtr virtPort,
> > +                                  int vf,
> > +                                  const unsigned char *vmuuid);
> > +
> > +static int disassociatePortProfileId(const char *macvtap_ifname,
> > +                                     const virVirtualPortProfileDefPtr virtPort);
> > +
> > +
> >  static int nlOpen(void)
> >  {
> >      int fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
> > @@ -567,39 +578,38 @@ configMacvtapTap(int tapfd, int vnet_hdr
> >  
> >      return 0;
> >  }
> > -
> > -
> >  /**
> >   * openMacvtapTap:
> >   * Create an instance of a macvtap device and open its tap character
> >   * device.
> >   * @tgifname: Interface name that the macvtap is supposed to have. May
> >   *    be NULL if this function is supposed to choose a name
> > - * @macaddress: The MAC address for the macvtap device
> > - * @linkdev: The interface name of the NIC to connect to the external bridge
> > - * @mode_str: String describing the mode. Valid are 'bridge', 'vepa' and
> > - *     'private'.
> > + * @net: pointer to the virDomainNetDef object describing the direct
> > + *       type if an interface
> >   * @res_ifname: Pointer to a string pointer where the actual name of the
> >   *     interface will be stored into if everything succeeded. It is up
> >   *     to the caller to free the string.
> > + * @vnet_hdr: Whether to enable IFF_VNET_HDR on the interface
> > + * @vmuuid: The (raw) UUID of the VM
> >   *
> >   * Returns file descriptor of the tap device in case of success,
> >   * negative value otherwise with error reported.
> >   *
> > + * Open a macvtap device and trigger the switch setup protocol
> > + * if valid port profile parameters were provided.
> >   */
> >  int
> >  openMacvtapTap(const char *tgifname,
> > -               const unsigned char *macaddress,
> > -               const char *linkdev,
> > -               int mode,
> > +               virDomainNetDefPtr net,
> 
> Any reason to keep tgifname now that you're passing full virDomainNetDefPtr?

True. Next would be to hanlde the **res_ifname in this function, but
that I'd leave to a separate patch.

> 
> >                 char **res_ifname,
> > -               int vnet_hdr)
> > +               int vnet_hdr,
> > +               const unsigned char *vmuuid)
> >  {
> >      const char *type = "macvtap";
> >      int c, rc;
> >      char ifname[IFNAMSIZ];
> >      int retries, do_retry = 0;
> > -    uint32_t macvtapMode = macvtapModeFromInt(mode);
> > +    uint32_t macvtapMode = macvtapModeFromInt(net->data.direct.mode);
> >      const char *cr_ifname;
> >      int ifindex;
> >  
> > @@ -616,7 +626,7 @@ openMacvtapTap(const char *tgifname,
> >              return -1;
> >          }
> >          cr_ifname = tgifname;
> > -        rc = link_add(type, macaddress, 6, tgifname, linkdev,
> > +        rc = link_add(type, net->mac, 6, tgifname, net->data.direct.linkdev,
> >                        macvtapMode, &do_retry);
> >          if (rc)
> >              return -1;
> > @@ -626,7 +636,8 @@ create_name:
> >          for (c = 0; c < 8192; c++) {
> >              snprintf(ifname, sizeof(ifname), MACVTAP_NAME_PATTERN, c);
> >              if (ifaceGetIndex(false, ifname, &ifindex) == ENODEV) {
> > -                rc = link_add(type, macaddress, 6, ifname, linkdev,
> > +                rc = link_add(type, net->mac, 6, ifname,
> > +                              net->data.direct.linkdev,
> >                                macvtapMode, &do_retry);
> >                  if (rc == 0)
> >                      break;
> > @@ -639,6 +650,14 @@ create_name:
> >          cr_ifname = ifname;
> >      }
> >  
> > +    if (associatePortProfileId(cr_ifname,
> > +                               &net->data.direct.virtPortProfile,
> > +                               -1,
> > +                               vmuuid) != 0) {
> > +        rc = -1;
> > +        goto link_del_exit;
> > +    }
> > +
> >      rc = ifaceUp(cr_ifname);
> >      if (rc != 0) {
> >          virReportSystemError(errno,
> > @@ -647,7 +666,7 @@ create_name:
> >                               "MAC address"),
> >                               cr_ifname);
> >          rc = -1;
> > -        goto link_del_exit;
> > +        goto disassociate_exit;
> >      }
> >  
> >      rc = openTap(cr_ifname, 10);
> > @@ -656,14 +675,18 @@ create_name:
> >          if (configMacvtapTap(rc, vnet_hdr) < 0) {
> >              close(rc);
> >              rc = -1;
> > -            goto link_del_exit;
> > +            goto disassociate_exit;
> >          }
> >          *res_ifname = strdup(cr_ifname);
> >      } else
> > -        goto link_del_exit;
> > +        goto disassociate_exit;
> >  
> >      return rc;
> >  
> > +disassociate_exit:
> > +    disassociatePortProfileId(cr_ifname,
> > +                              &net->data.direct.virtPortProfile);
> > +
> >  link_del_exit:
> >      link_del(cr_ifname);
> >  
> > @@ -673,14 +696,102 @@ link_del_exit:
> >  
> >  /**
> >   * delMacvtap:
> > - * @ifname : The name of the macvtap interface
> > + * @net: pointer to virDomainNetDef object
> >   *
> > - * Delete an interface given its name.
> > + * Delete an interface given its name. Disassociate
> > + * it with the switch if port profile parameters
> > + * were provided.
> >   */
> >  void
> > -delMacvtap(const char *ifname)
> > +delMacvtap(virDomainNetDefPtr net)
> >  {
> > -    link_del(ifname);
> > +    if (net->ifname) {
> > +        disassociatePortProfileId(net->ifname,
> > +                                  &net->data.direct.virtPortProfile);
> > +        link_del(net->ifname);
> > +    }
> >  }
> >  
> >  #endif
> > +
> > +
> > +/**
> > + * associatePortProfile
> > + *
> > + * @macvtap_ifname: The name of the macvtap device
> > + * @virtPort: pointer to the object holding port profile parameters
> > + * @vf: virtual function number, -1 if to be ignored
> > + * @vmuuid : the UUID of the virtual machine
> > + *
> > + * Associate a port on a swtich with a profile. This function
> > + * may notify a kernel driver or an external daemon to run
> > + * the setup protocol. If profile parameters were not supplied
> > + * by the user, then this function returns without doing
> > + * anything.
> > + *
> > + * Returns 0 in case of success, != 0 otherwise with error
> > + * having been reported.
> > + */
> > +static int
> > +associatePortProfileId(const char *macvtap_ifname,
> > +                       const virVirtualPortProfileDefPtr virtPort,
> > +                       int vf,
> > +                       const unsigned char *vmuuid)
> > +{
> > +    int rc = 0;
> > +    VIR_DEBUG("Associating port profile '%p' on link device '%s'",
> > +              virtPort, macvtap_ifname);
> > +    (void)vf;
> > +    (void)vmuuid;
> > +
> > +    switch (virtPort->virtPortType) {
> > +    case VIR_VIRTUALPORT_NONE:
> > +    case VIR_VIRTUALPORT_TYPE_LAST:
> > +        break;
> 
> If we capture these on parsing and error out, then these could be
> invalid here.

They would not even appear here. Problem is just I cannot even test
anything with macvtap anymore without having a profile which then in
turn requires either a daemon or an enic kernel driver.

> 
> > +
> > +    case VIR_VIRTUALPORT_8021QBG:
> > +
> > +        break;
> > +
> > +    case VIR_VIRTUALPORT_8021QBH:
> > +
> > +        break;
> > +    }
> > +
> > +    return rc;
> > +}
> > +
> > +
> > +/**
> > + * disassociatePortProfile
> > + *
> > + * @macvtap_ifname: The name of the macvtap device
> > + * @virtPort: point to object holding port profile parameters
> > + *
> > + * Returns 0 in case of success, != 0 otherwise with error
> > + * having been reported.
> > + */
> > +static int
> > +disassociatePortProfileId(const char *macvtap_ifname,
> > +                          const virVirtualPortProfileDefPtr virtPort)
> > +{
> > +    int rc = 0;
> > +    VIR_DEBUG("Disassociating port profile id '%p' on link device '%s' ",
> > +              virtPort, macvtap_ifname);
> > +
> > +    switch (virtPort->virtPortType) {
> > +    case VIR_VIRTUALPORT_NONE:
> > +    case VIR_VIRTUALPORT_TYPE_LAST:
> > +        break;
> 
> If we capture these on parsing and error out, then these could be
> invalid here.
> 

If I do that then please send me hardware :-)

I'll post v8 after some testing - while I still can :-)

   Stefan



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