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

[Chris Wright]

* Hugh O. Brock (hbrock at redhat.com) wrote:
> On Mon, May 24, 2010 at 11:47:18AM -0700, Chris Wright wrote:
> > * Daniel P. Berrange (berrange at redhat.com) wrote:
> > > On Sun, May 23, 2010 at 12:51:50PM -0400, Stefan Berger wrote:
> > > > Index: libvirt-acl/src/util/macvtap.h
> > > > ===================================================================
> > > > --- libvirt-acl.orig/src/util/macvtap.h
> > > > +++ libvirt-acl/src/util/macvtap.h
> > > > @@ -27,15 +27,14 @@
> > > >  # if defined(WITH_MACVTAP)
> > > >  
> > > >  #  include "internal.h"
> > > > +#  include "conf/domain_conf.h"
> > > 
> > > This isn't allowed. It is introducing a dependancy cycle
> > > between the util & conf directories. Code in util/ is not
> > > allowed to depend on any other code in the libvirt tree.
> > 
> > IOW, you mean using virDomainNetDefPtr in openMacvtapTap is a libvirt
> > layering violation, and you'd prefer openMacvtapTap() w/ large number
> > of parameters?  I think it's impractical to not invent some structure to
> > pass the data...otherwise, I believe the worst case would be:
> > 
> > int openMacvtapTap(const char *tgifname,
> > 		   const unsigned char *macaddress,
> > 		   const char *linkdev,
> > 		   int mode,
> > 		   char **res_ifname,
> > 		   int vnet_hdr,
> > 		   int vf,
> > 		   int port_type,
> > 		   unsigned char mgrid,
> > 		   unsigned typeid,
> > 		   const unsigned char *instanceid,
> > 		   const unsigned char *profileid,
> > 		   const unsigned char *vmuuid)
> > 
> > But, any such structure will create some dependency.
> > 
> > What do you think?
> > 
> > thanks,
> > -chris
> 
> Earlier today on IRC Dan said:
> 
> <danpb> if that's really neccessary, a macvtap.h should have its own
> struct definition, separate from the XML structs.
> 
> Hopefully that makes sense in this context?

It does.  The only question is how to share the structure definition.
For example, we have:

typedef struct _virVirtualPortProfileDef virVirtualPortProfileDef;
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;
};

So we either promote the union or structures within virVirtualPortProfileDef
to a common def and pass those, or create some new def to share.

BTW

$ grep conf/ src/util/*.[ch]
src/util/hooks.c:#include "conf/domain_conf.h"
src/util/macvtap.c:# include "conf/domain_conf.h"

thanks,
-chris