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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]