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

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



On Mon, May 10, 2010 at 07:57:37PM -0400, Stefan Berger wrote:
> Below is David Alan's original patch with lots of changes. 
> 
> In particular, it now parses the following XML and stored the data
> internally. No sending of netlink messages has been implemented here.
> 
>    <interface type='direct'>
>       <source dev='static' mode='vepa'/>
>       <model type='virtio'/>
>       <vsi managerid='12' typeid='0x123456' typeidversion='1'
>            instanceid='fa9b7fff-b0a0-4893-8e0e-beef4ff18f8f' />
>       <filterref filter='clean-traffic'/>
>     </interface>
> 
>     <interface type='direct'>
>       <source dev='static' mode='vepa'/>
>       <model type='virtio'/>
>       <vsi profileid='my_profile'/>
>     </interface>

Could we have an explanation of what these attributes all mean in
the commit message. 

Also, when we have 2 different sets of attributes, we normally use
a type attribute on the element to tell the parser what set of
data to expect. So I think this should gain a 'type' attribute here.

> @@ -1873,6 +1879,20 @@ virDomainNetDefParseXML(virCapsPtr caps,
>                         xmlStrEqual(cur->name, BAD_CAST "source")) {
>                  dev  = virXMLPropString(cur, "dev");
>                  mode = virXMLPropString(cur, "mode");
> +            } else if ((vsiManagerID == NULL) &&
> +                       (vsiTypeID == NULL) &&
> +                       (vsiTypeIDVersion == NULL) &&
> +                       (vsiInstanceID == NULL) &&
> +                       (vsiProfileID == NULL) &&
> +                       (def->type == VIR_DOMAIN_NET_TYPE_DIRECT) &&
> +                       xmlStrEqual(cur->name, BAD_CAST "vsi")) {
> +                vsiManagerID = virXMLPropString(cur, "managerid");
> +                vsiTypeID = virXMLPropString(cur, "typeid");
> +                vsiTypeIDVersion = virXMLPropString(cur,
> "typeidversion");
> +                vsiInstanceID = virXMLPropString(cur, "instanceid");
> +#ifdef IFLA_VF_PORT_PROFILE_MAX
> +                vsiProfileID = virXMLPropString(cur, "profileid");
> +#endif

XML parsing routines should not be #ifdefd. The XML format is formally
defined by the schema and must never change based on compile time options.

>              } else if ((network == NULL) &&
>                         ((def->type == VIR_DOMAIN_NET_TYPE_SERVER) ||
>                          (def->type == VIR_DOMAIN_NET_TYPE_CLIENT) ||
> @@ -2049,6 +2069,51 @@ virDomainNetDefParseXML(virCapsPtr caps,
>          } else
>              def->data.direct.mode =
> VIR_DOMAIN_NETDEV_MACVTAP_MODE_VEPA;
>  
> +        vsi = &def->data.direct.vsiProfile;
> +
> +#ifdef IFLA_VF_PORT_PROFILE_MAX
> +        if (vsiProfileID != NULL) {
> +            if (virStrcpyStatic(vsi->u.vsi8021Qbh.profileID,
> +                                vsiProfileID) != NULL) {
> +                vsi->vsiType = VIR_VSI_8021QBH;
> +                break;
> +            }
> +        }
> +#endif

Likewise here

> +
> +        while (vsiManagerID     != NULL && vsiTypeID     != NULL &&
> +               vsiTypeIDVersion != NULL && vsiInstanceID != NULL) {
> +            unsigned int val;
> +
> +            if ((virStrToLong_ui(vsiManagerID, NULL, 10, &val) &&
> +                 virStrToLong_ui(vsiManagerID, NULL, 16, &val)   ) ||
> +                val > 0xff)
> +                break;
> +
> +            vsi->u.vsi8021Qbg.managerID = (uint8_t)val;
> +
> +            if ((virStrToLong_ui(vsiTypeID, NULL, 10, &val) &&
> +                 virStrToLong_ui(vsiTypeID, NULL, 16, &val)   ) ||
> +                val > 0xffffff)
> +                break;
> +
> +            vsi->u.vsi8021Qbg.typeID = (uint32_t)val;
> +
> +            if ((virStrToLong_ui(vsiTypeIDVersion, NULL, 10, &val) &&
> +                 virStrToLong_ui(vsiTypeIDVersion, NULL, 16, &val)   )
> ||
> +                val > 0xff)
> +                break;
> +
> +            vsi->u.vsi8021Qbg.typeIDVersion = (uint8_t)val;
> +
> +            if (virUUIDParse(vsiInstanceID,
> +
> def->data.direct.vsiProfile.u.vsi8021Qbg.instanceID))
> +                break;
> +
> +            vsi->vsiType = VIR_VSI_8021QBG;
> +            break;
> +        }
> +
>          def->data.direct.linkdev = dev;
>          dev = NULL;
>  
> @@ -2114,6 +2179,11 @@ cleanup:
>      VIR_FREE(internal);
>      VIR_FREE(devaddr);
>      VIR_FREE(mode);
> +    VIR_FREE(vsiManagerID);
> +    VIR_FREE(vsiTypeID);
> +    VIR_FREE(vsiTypeIDVersion);
> +    VIR_FREE(vsiInstanceID);
> +    VIR_FREE(vsiProfileID);
>      virNWFilterHashTableFree(filterparams);
>  
>      return def;
> @@ -5076,6 +5146,8 @@ virDomainNetDefFormat(virBufferPtr buf,
>  {
>      const char *type = virDomainNetTypeToString(def->type);
>      char *attrs;
> +    char uuidstr[VIR_UUID_STRING_BUFLEN];
> +    virVSIProfileDefPtr vsi;
>  
>      if (!type) {
>          virDomainReportError(VIR_ERR_INTERNAL_ERROR,
> @@ -5141,6 +5213,31 @@ virDomainNetDefFormat(virBufferPtr buf,
>          virBufferVSprintf(buf, " mode='%s'",
> 
> virDomainNetdevMacvtapTypeToString(def->data.direct.mode));
>          virBufferAddLit(buf, "/>\n");
> +        vsi = &def->data.direct.vsiProfile;
> +        switch (vsi->vsiType) {
> +        case VIR_VSI_INVALID:
> +            break;
> +
> +        case VIR_VSI_8021QBG:
> +
> virUUIDFormat(def->data.direct.vsiProfile.u.vsi8021Qbg.instanceID,
> +                          uuidstr);
> +            virBufferVSprintf(buf,
> +                              "      <vsi managerid='%d' typeid='%d' "
> +                              "typeidversion='%d' instanceid='%s'/>\n",
> +                              vsi->u.vsi8021Qbg.managerID,
> +                              vsi->u.vsi8021Qbg.typeID,
> +                              vsi->u.vsi8021Qbg.typeIDVersion,
> +                              uuidstr);
> +            break;
> +
> +#ifdef IFLA_VF_PORT_PROFILE_MAX
> +        case VIR_VSI_8021QBH:
> +            virBufferVSprintf(buf,
> +                              "      <vsi profileid='%s'/>\n",
> +                              vsi->u.vsi8021Qbh.profileID);
> +            break;
> +#endif
> +        }
>          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,36 @@ enum virDomainNetdevMacvtapType {
>  };
>  
>  
> +#define IFLA_VF_PORT_PROFILE_MAX 40
> +enum virVSIType {
> +    VIR_VSI_INVALID,

This isn't really 'INVALID' - this is better named 'NONE'
since its simply an indication that this interface does not
have any VSI info defined

> +    VIR_VSI_8021QBG,
> +#ifdef IFLA_VF_PORT_PROFILE_MAX
> +    VIR_VSI_8021QBH,
> +#endif

And here, etc

> +};
> +
> +/* profile data for macvtap (VEPA) */
> +typedef struct _virVSIProfileDef virVSIProfileDef;
> +typedef virVSIProfileDef *virVSIProfileDefPtr;
> +struct _virVSIProfileDef {
> +    enum virVSIType   vsiType;
> +    union {
> +        struct {
> +            uint8_t       managerID;
> +            uint32_t      typeID; // 24 bit valid
> +            uint8_t       typeIDVersion;
> +            unsigned char instanceID[VIR_UUID_BUFLEN];
> +        } vsi8021Qbg;
> +#ifdef IFLA_VF_PORT_PROFILE_MAX
> +        struct {
> +            char          profileID[IFLA_VF_PORT_PROFILE_MAX];
> +        } vsi8021Qbh;
> +#endif
> +    } u;
> +};

> Index: libvirt-acl/tests/domainschemadata/portprofile.xml
> ===================================================================
> --- /dev/null
> +++ libvirt-acl/tests/domainschemadata/portprofile.xml
> @@ -0,0 +1,27 @@
> +<domain type='lxc'>
> +  <name>portprofile</name>
> +  <uuid>00000000-0000-0000-0000-000000000000</uuid>
> +  <memory>1048576</memory>
> +    <os>
> +        <type>exe</type>
> +        <init>/sh</init>
> +    </os>
> +  <devices>
> +    <interface type='direct'>
> +      <source dev='eth0' mode='vepa'/>
> +      <vsi managerid='12' typeid='1193046' typeidversion='1'
> +      instanceid='fa9b7fff-b0a0-4893-8e0e-beef4ff18f8f'/>
> +    </interface>
> +    <interface type='direct'>
> +      <source dev='eth0' mode='vepa'/>
> +      <vsi profileid='my_profile'/>
> +    </interface>
> +    <interface type='direct'>
> +      <source dev='eth0' mode='vepa'/>
> +      <vsi/>
> +    </interface>
> +    <interface type='direct'>
> +      <source dev='eth0' mode='vepa'/>
> +    </interface>
> +  </devices>
> +</domain>
> Index: libvirt-acl/src/qemu/qemu_conf.h
> ===================================================================
> --- libvirt-acl.orig/src/qemu/qemu_conf.h
> +++ libvirt-acl/src/qemu/qemu_conf.h
> @@ -271,8 +271,6 @@ qemudOpenVhostNet(virDomainNetDefPtr net
>  int qemudPhysIfaceConnect(virConnectPtr conn,
>                            struct qemud_driver *driver,
>                            virDomainNetDefPtr net,
> -                          char *linkdev,
> -                          int brmode,
>                            unsigned long long qemuCmdFlags);
>  
>  int         qemudProbeMachineTypes      (const char *binary,
> Index: libvirt-acl/src/qemu/qemu_driver.c
> ===================================================================
> --- libvirt-acl.orig/src/qemu/qemu_driver.c
> +++ libvirt-acl/src/qemu/qemu_driver.c
> @@ -3585,10 +3585,8 @@ static void qemudShutdownVMDaemon(struct
>      def = vm->def;
>      for (i = 0; i < def->nnets; i++) {
>          virDomainNetDefPtr net = def->nets[i];
> -        if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
> -            if (net->ifname)
> -                delMacvtap(net->ifname);
> -        }
> +        if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT)
> +            delMacvtap(net);
>      }
>  #endif
>  
> @@ -7175,8 +7173,6 @@ static int qemudDomainAttachNetDevice(vi
>          }
>  
>          if ((tapfd = qemudPhysIfaceConnect(conn, driver, net,
> -                                           net->data.direct.linkdev,
> -                                           net->data.direct.mode,
>                                             qemuCmdFlags)) < 0)
>              return -1;
>      }
> @@ -8146,10 +8142,8 @@ qemudDomainDetachNetDevice(struct qemud_
>      virNWFilterTearNWFilter(detach);
>  
>  #if WITH_MACVTAP
> -    if (detach->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
> -        if (detach->ifname)
> -            delMacvtap(detach->ifname);
> -    }
> +    if (detach->type == VIR_DOMAIN_NET_TYPE_DIRECT)
> +        delMacvtap(detach);
>  #endif
>  
>      if ((driver->macFilter) && (detach->ifname != NULL)) {


Regards,
Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


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