[libvirt] [PATCH] Fix a crash when using Open vSwitch virtual ports
Alex Jia
ajia at redhat.com
Thu Aug 30 06:36:12 UTC 2012
On 08/30/2012 01:49 PM, Daniel Veillard wrote:
Daniel, BTW, To test this patch again, it works for me :)
> On Wed, Aug 29, 2012 at 02:44:36PM -0400, Kyle Mestery wrote:
>> Fixup buffer usage when handling VLANs. Also fix the logic
>> used to determine if the virNetDevVlanPtr is valid or not.
>> Fixes crashes in the latest code when using Open vSwitch
>> virtualports.
>>
>> Signed-off-by: Kyle Mestery<kmestery at cisco.com>
>> ---
>> src/util/virnetdevopenvswitch.c | 26 +++++++++++++-------------
>> 1 file changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
>> index b903ae4..cdbc5ef 100644
>> --- a/src/util/virnetdevopenvswitch.c
>> +++ b/src/util/virnetdevopenvswitch.c
>> @@ -59,7 +59,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname,
>> char *ifaceid_ex_id = NULL;
>> char *profile_ex_id = NULL;
>> char *vmid_ex_id = NULL;
>> - virBufferPtr buf;
>> + virBuffer buf = VIR_BUFFER_INITIALIZER;
>>
>> virMacAddrFormat(macaddr, macaddrstr);
>> virUUIDFormat(ovsport->interfaceID, ifuuidstr);
>> @@ -79,13 +79,12 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname,
>> ovsport->profileID)< 0)
>> goto out_of_memory;
>> }
>> - if (virtVlan) {
>> - if (VIR_ALLOC(buf)< 0)
>> - goto out_of_memory;
>> +
>> + if (virtVlan&& virtVlan->nTags> 0) {
>>
>> /* Trunk port first */
>> - if (virtVlan->trunk) {
>> - virBufferAddLit(buf, "trunk=");
>> + if (virtVlan->trunk == true) {
>> + virBufferAddLit(&buf, "trunk=");
>>
>> /*
>> * Trunk ports have at least one VLAN. Do the first one
>> @@ -93,21 +92,21 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname,
>> * start of the for loop if there are more than one VLANs
>> * on this trunk port.
>> */
>> - virBufferAsprintf(buf, "%d", virtVlan->tag[i]);
>> + virBufferAsprintf(&buf, "%d", virtVlan->tag[i]);
>>
>> for (i = 1; i< virtVlan->nTags; i++) {
>> - virBufferAddLit(buf, ",");
>> - virBufferAsprintf(buf, "%d", virtVlan->tag[i]);
>> + virBufferAddLit(&buf, ",");
>> + virBufferAsprintf(&buf, "%d", virtVlan->tag[i]);
>> }
>> } else if (virtVlan->nTags) {
>> - virBufferAsprintf(buf, "tag=%d", virtVlan->tag[0]);
>> + virBufferAsprintf(&buf, "tag=%d", virtVlan->tag[0]);
>> }
>> }
>>
>> cmd = virCommandNew(OVSVSCTL);
>> if (ovsport->profileID[0] == '\0') {
>> virCommandAddArgList(cmd, "--", "--may-exist", "add-port",
>> - brname, ifname, virBufferContentAndReset(buf),
>> + brname, ifname, virBufferCurrentContent(&buf),
>> "--", "set", "Interface", ifname, attachedmac_ex_id,
>> "--", "set", "Interface", ifname, ifaceid_ex_id,
>> "--", "set", "Interface", ifname, vmid_ex_id,
>> @@ -116,7 +115,7 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname,
>> NULL);
>> } else {
>> virCommandAddArgList(cmd, "--", "--may-exist", "add-port",
>> - brname, ifname, virBufferContentAndReset(buf),
>> + brname, ifname, virBufferCurrentContent(&buf),
>> "--", "set", "Interface", ifname, attachedmac_ex_id,
>> "--", "set", "Interface", ifname, ifaceid_ex_id,
>> "--", "set", "Interface", ifname, vmid_ex_id,
>> @@ -135,7 +134,8 @@ int virNetDevOpenvswitchAddPort(const char *brname, const char *ifname,
>>
>> ret = 0;
>> cleanup:
>> - VIR_FREE(buf);
>> + if (virBufferUse(&buf)> 0)
>> + virBufferFreeAndReset(&buf);
> that looks fine up to here, where we could leak in theory,
> virBufferUse() return the amount of bytes used in the buffer, not
> if the buffer was allocated. Sounds to me we can use
> virBufferFreeAndReset() directly without the test
>
>> VIR_FREE(attachedmac_ex_id);
>> VIR_FREE(ifaceid_ex_id);
>> VIR_FREE(vmid_ex_id);
> I pushed with that small change,
>
> thanks !
>
> Daniel
>
More information about the libvir-list
mailing list