[libvirt] [PATCH] Fix a crash when using Open vSwitch virtual ports

[Alex Jia]

On 08/30/2012 02:44 AM, 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.

Hi Kyle,
I just waive my patch then apply Laine and your patch, it works well for 
me now,
so give a ACK.

Regards,
Alex

> 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);
>       VIR_FREE(attachedmac_ex_id);
>       VIR_FREE(ifaceid_ex_id);
>       VIR_FREE(vmid_ex_id);