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

* Hugh O. Brock (hbrock(a)redhat.com) wrote: > On Mon, May 24, 2010 at 11:47:18AM -0700, Chris Wright wrote: > > * Daniel P. Berrange (berrange(a)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.

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