Re: [Qemu-devel] [PATCH v4 1/4] Add basic version of bridge helper

[Corey Bryant]

On 11/03/2011 03:54 AM, Stefan Hajnoczi wrote:
> On Wed, Nov 2, 2011 at 3:12 PM, Corey Bryant<address@hidden>  wrote:
> > On 11/02/2011 06:58 AM, Stefan Hajnoczi wrote:
> > > On Tue, Nov 1, 2011 at 5:13 PM, Corey Bryant<address@hidden>
> > >   wrote:
> > > > +static bool has_vnet_hdr(int fd)
> > > > +{
> > > > +    unsigned int features = 0;
> > > > +    struct ifreq ifreq;
> > > > +
> > > > +    if (ioctl(fd, TUNGETFEATURES,&features) == -1) {
> > > > +        return false;
> > > > +    }
> > > > +
> > > > +    if (!(features&    IFF_VNET_HDR)) {
> > > > +        return false;
> > > > +    }
> > > > +
> > > > +    if (ioctl(fd, TUNGETIFF,&ifreq) != -1 || errno != EBADFD) {
> > > > +        return false;
> > > > +    }
> > >
> > > I don't understand this expression.  We want TUNGETIFF to fail with
> > > EBADFD, otherwise we return false.  What is this trying to do?
> > >
> > > Why do we even need TUNGETIFF after TUNGETFEATURES succeeded and we
> > > were able to check out the IFF_VNET_HDR flag?
> > >
> > > Stefan
> >
> > I don't think the TUNGETIFF call is necessary.  It verifies that the tap
> > device doesn't already exist.  The ensuing TUNSETIFF call should already
> > cover this.
>
> Thanks for explaining.  It would be nice to comment this or drop the
> if statement entirely if you are sending another revision of this
> patch series.
>
> Stefan

You're welcome, and thanks for the comment.  I'll drop this if statement 
in the next version of the patch series.

Regards,
Corey