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

[Corey Bryant <coreyb@linux.vnet.ibm.com>]

On 11/03/2011 03:54 AM, Stefan Hajnoczi wrote:
> On Wed, Nov 2, 2011 at 3:12 PM, Corey Bryant<coreyb@linux.vnet.ibm.com>  wrote:
>> On 11/02/2011 06:58 AM, Stefan Hajnoczi wrote:
>>>
>>> On Tue, Nov 1, 2011 at 5:13 PM, Corey Bryant<coreyb@linux.vnet.ibm.com>
>>>   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