On Mon, May 24, 2010 at 01:12:12PM -0700, Chris Wright wrote:
> * Hugh O. Brock (hbrock@xxxxxxxxxx) wrote:
> > On Mon, May 24, 2010 at 11:47:18AM -0700, Chris Wright wrote:
> > > * Daniel P. Berrange (berrange@xxxxxxxxxx) 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.
Yep, just moving this struct into macvtap.h would be sufficient
for now. Also rename it to virVirtualPortProfileParams while doing
this, since names ending in 'Def' are specifically related to the
XML parser routines.
> $ grep conf/ src/util/*.[ch]
> src/util/hooks.c:#include "conf/domain_conf.h"
> src/util/macvtap.c:# include "conf/domain_conf.h"
Both bugs that must be fixed. The include in hooks.c doesn't appear to be
needed at all - it compiles fine without it. Likewise macvtap.c compiles
fine without it for me too.
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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list