[libvirt] [PATCHv4] Configure native vlan modes on Open vSwitch ports

[John Ferlan]

On 06/17/2013 01:56 PM, james robson wrote:

<...snip...>
>> diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
>> index 2aee445..47e6027 100644
>> --- a/src/util/virnetdevopenvswitch.c
>> +++ b/src/util/virnetdevopenvswitch.c
>> @@ -109,8 +109,22 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname,
>>      virCommandAddArgList(cmd, "--timeout=5", "--", "--may-exist", "add-port",
>>                          brname, ifname, NULL);
>>  
>> -    if (virBufferUse(&buf) != 0)
>> +    if (virBufferUse(&buf) != 0) {
>> +        switch (virtVlan->nativeMode) {
>> +            case VIR_NATIVE_VLAN_MODE_TAGGED:
>> +                virCommandAddArg(cmd, "vlan_mode=native-tagged");
>> +                virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag);
>> +                break;
>> +            case VIR_NATIVE_VLAN_MODE_UNTAGGED:
>> +                virCommandAddArg(cmd, "vlan_mode=native-untagged");
>> +                virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag);
>> +                break;
>> +            case VIR_NATIVE_VLAN_MODE_DEFAULT:
>> +            default:
>> +                break;
>> +        }
>>          virCommandAddArgList(cmd, virBufferCurrentContent(&buf), NULL);
>> +    }

My overnight Coverity run has found the following problem with above code.
The issue is that at line 84 there's an "if (virtVlan &&" condition; whereas,
this reference at line 112 doesn't check virtVlan before referencing:

68   	

(1) Event cond_false: 	Condition "virAsprintf(&attachedmac_ex_id, "external-ids:attached-mac=\"%s\"", macaddrstr) < 0", taking false branch

69   	    if (virAsprintf(&attachedmac_ex_id, "external-ids:attached-mac=\"%s\"",
70   	                    macaddrstr) < 0)
71   	        goto out_of_memory;

(2) Event cond_false: 	Condition "virAsprintf(&ifaceid_ex_id, "external-ids:iface-id=\"%s\"", ifuuidstr) < 0", taking false branch

72   	    if (virAsprintf(&ifaceid_ex_id, "external-ids:iface-id=\"%s\"",
73   	                    ifuuidstr) < 0)
74   	        goto out_of_memory;

(3) Event cond_false: 	Condition "virAsprintf(&vmid_ex_id, "external-ids:vm-id=\"%s\"", vmuuidstr) < 0", taking false branch

75   	    if (virAsprintf(&vmid_ex_id, "external-ids:vm-id=\"%s\"",
76   	                    vmuuidstr) < 0)
77   	        goto out_of_memory;

(4) Event cond_true: 	Condition "ovsport->profileID[0] != 0", taking true branch

78   	    if (ovsport->profileID[0] != '\0') {

(5) Event cond_false: 	Condition "virAsprintf(&profile_ex_id, "external-ids:port-profile=\"%s\"", ovsport->profileID) < 0", taking false branch

79   	        if (virAsprintf(&profile_ex_id, "external-ids:port-profile=\"%s\"",
80   	                        ovsport->profileID) < 0)
81   	            goto out_of_memory;
82   	    }
83   	

(6) Event cond_false: 	Condition "virtVlan", taking false branch
(8) Event var_compare_op: 	Comparing "virtVlan" to null implies that "virtVlan" might be null.
Also see events: 	[var_deref_op]

84   	    if (virtVlan && virtVlan->nTags > 0) {
85   	
86   	        /* Trunk port first */
87   	        if (virtVlan->trunk) {
88   	            virBufferAddLit(&buf, "trunk=");
89   	
90   	            /*
91   	             * Trunk ports have at least one VLAN. Do the first one
92   	             * outside the "for" loop so we can put a "," at the
93   	             * start of the for loop if there are more than one VLANs
94   	             * on this trunk port.
95   	             */
96   	            virBufferAsprintf(&buf, "%d", virtVlan->tag[i]);
97   	
98   	            for (i = 1; i < virtVlan->nTags; i++) {
99   	                virBufferAddLit(&buf, ",");
100  	                virBufferAsprintf(&buf, "%d", virtVlan->tag[i]);
101  	            }
102  	        } else if (virtVlan->nTags) {
103  	            virBufferAsprintf(&buf, "tag=%d", virtVlan->tag[0]);
104  	        }

(7) Event if_end: 	End of if statement

105  	    }
106  	
107  	    cmd = virCommandNew(OVSVSCTL);
108  	
109  	    virCommandAddArgList(cmd, "--timeout=5", "--", "--may-exist", "add-port",
110  	                        brname, ifname, NULL);
111  	

(9) Event var_deref_op: 	Dereferencing null pointer "virtVlan".
Also see events: 	[var_compare_op]

112  	    switch (virtVlan->nativeMode) {
113  	    case VIR_NATIVE_VLAN_MODE_TAGGED:
114  	        virCommandAddArg(cmd, "vlan_mode=native-tagged");
115  	        virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag);
116  	        break;
117  	    case VIR_NATIVE_VLAN_MODE_UNTAGGED:
118  	        virCommandAddArg(cmd, "vlan_mode=native-untagged");
119  	        virCommandAddArgFormat(cmd, "tag=%d", virtVlan->nativeTag);
120  	        break;
121  	    case VIR_NATIVE_VLAN_MODE_DEFAULT:
122  	    default:
123  	        break;
124  	    }

<...snip...>