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.
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