[libvirt] [PATCH] networkRunHook: Run hook only if possible

[John Ferlan]

On 02/19/2014 09:54 AM, Michal Privoznik wrote:
> Currently, networkRunHook() is called in networkAllocateActualDevice and
> friends. These functions, however, doesn't necessarily work on networks,

s/doesn't/don't/

> For example, if domain's interface is defined in this fashion:
> 
>     <interface type='bridge'>
>       <mac address='52:54:00:0b:3b:16'/>
>       <source bridge='virbr1'/>
>       <model type='rtl8139'/>
>       <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/>
>     </interface>
> 
> The networkAllocateActualDevice jumps directly onto 'validate' label as
> the interface is not type of 'network'. Hence, @network is left
> initialized to NULL and networkRunHook(network, ...) is called. One of
> the things that the hook function does is dereference @network. Soupir.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/network/bridge_driver.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
I'll give an ACK to the change...

Although I will also note that while the patch does resolve the current
issue and perhaps protect against future errant callers... Currently the
only caller that could get into the hook function with network == NULL
is networkAllocateActualDevice()

Other places that would end up using the hook will bail when iface->type
!= VIR_DOMAIN_NET_TYPE_NETWORK or when/by checking
virNetworkObjIsActive(network).

Your other option would seem to be checking "(iface->type ==
VIR_DOMAIN_NET_TYPE_NETWORK)" before calling the hook in the validate:
section.

John


> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index ee264b9..a6c719d 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -150,6 +150,12 @@ networkRunHook(virNetworkObjPtr network,
>      int ret = -1;
>  
>      if (virHookPresent(VIR_HOOK_DRIVER_NETWORK)) {
> +        if (!network) {
> +            VIR_DEBUG("Not running hook as @network is NULL");
> +            ret = 0;
> +            goto cleanup;
> +        }
> +
>          virBufferAddLit(&buf, "<hookData>\n");
>          virBufferAdjustIndent(&buf, 2);
>          if (virNetworkDefFormatBuf(&buf, network->def, 0) < 0)
>