[libvirt] [PATCH 1/5] network: Resolve some issues around vlan copying

[Laine Stump]

On 01/15/2013 01:35 PM, John Ferlan wrote:
> Remove extraneous check for 'netdef' when dereferencing for vlan.nTags.
> Prior code would already check if netdef was NULL.
>
> Coverity complained about a path where the 'vlan' was potentially valid,
> but a prior checks may not have allocated 'iface->data.network.actual',
> so like other paths it needs to be allocated on the fly.
> ---
>  src/network/bridge_driver.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index f1be954..e2b8d06 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -4005,11 +4005,21 @@ networkAllocateActualDevice(virDomainNetDefPtr iface)
>          vlan = &iface->vlan;
>      else if (portgroup && portgroup->vlan.nTags > 0)
>          vlan = &portgroup->vlan;
> -    else if (netdef && netdef->vlan.nTags > 0)
> +    else if (netdef->vlan.nTags > 0)
>          vlan = &netdef->vlan;

Correct. If netdef was NULL, that would mean (iface->type !=
VIR_DOMAIN_NET_TYPE_NETWORK), and in that case we would have skipped
over this code down to validate:

> -    if (virNetDevVlanCopy(&iface->data.network.actual->vlan, vlan) < 0)
> -        goto error;
> +    if (vlan) {
> +        /* data.network.actual may be NULL here when netdef->foward.type is
> +         * VIR_NETWORK_FORWARD_{NONE|NAT|ROUTE}
> +         */
> +        if (!iface->data.network.actual
> +            && (VIR_ALLOC(iface->data.network.actual) < 0)) {
> +            virReportOOMError();
> +            goto error;
> +        }

Hmm. This ends up giving us an actual with no type set, which tells me
we should have allocated this prior to now, which points out that I
added this bit of code in the wrong place. I'll send an alternate patch
momentarily.

> +        if (virNetDevVlanCopy(&iface->data.network.actual->vlan, vlan) < 0)
> +            goto error;
> +    }
>  
>  validate:
>      /* make sure that everything now specified for the device is