[libvirt] [PATCH] Fix adding ports to OVS bridges without VLAN tags

Kyle Mestery (kmestery) kmestery at cisco.com
Tue Sep 4 21:03:22 UTC 2012


On Aug 31, 2012, at 9:09 AM, Daniel Veillard wrote:
> On Fri, Aug 31, 2012 at 01:32:34PM +0000, Kyle Mestery (kmestery) wrote:
>> On Aug 30, 2012, at 10:23 PM, Daniel Veillard wrote:
>>> On Thu, Aug 30, 2012 at 04:38:06PM -0400, Kyle Mestery wrote:
> [...]
>>> Still there is something which looks wrong, if we don't have a profileID
>>> why do we end up with "" instead of NULL ? I'm seeing various tests for
>>> profileID[0] over conf/*.c and util/*.c , and that sounds wrong to me.
>>> if there is no data, store NULL ! Then test for profileID instead of
>>> profileID[0]. Then there is no risk of a crash because abscence of data
>>> led to NULL instead of an empty string, the code is more resilient !
>>> 
>>> I expect a followup patch cleaning this up, but after 0.10.1 ...
>>> thanks !
>>> 
>> Thanks Daniel, I'll work on the followup patch today.
> 
>  No hurry, because I just rolled 0.10.1 out so that won't make it
> (and it's not urgent). Giving 0.10.1 a try would be nice too,
> 
>  thanks !
> 
> Daniel


Hi Daniel:

Picking this back up. struct _virNetDevVPortProfile  contains the following:

    /* this member is used when virtPortType == 802.1Qbh|openvswitch */
    /* this is a null-terminated character string */
    char          profileID[LIBVIRT_IFLA_VF_PORT_PROFILE_MAX];

To address your comments around checking for profileID[0], we could make
profileID here a pointer, and allocate it when we allocate a struct _virNetDevVPortProfile
object. But to me, having a fixed length string in this structure doesn't seem wrong.
Copying Laine here as well for his comments, but I'm inclined to leave things as they
are.

Thanks,
Kyle




More information about the libvir-list mailing list