[libvirt] [PATCH 05/16] network: Move macmap mgmt from bridge_driver to virnetworkobj

[Pavel Hrdina]

On Fri, May 19, 2017 at 09:03:13AM -0400, John Ferlan wrote:
> In preparation for having a private virNetworkObj - let's create/move some
> API's that handle the obj->macmap. The API's will be renamed to have a
> virNetworkObj prefix to follow conventions and the arguments slightly
> modified to accept what's necessary to complete their task.
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/conf/virnetworkobj.c    |  97 ++++++++++++++++++++++++++++++++++++++++++
>  src/conf/virnetworkobj.h    |  26 ++++++++++++
>  src/libvirt_private.syms    |   6 +++
>  src/network/bridge_driver.c | 101 ++++++++------------------------------------
>  4 files changed, 147 insertions(+), 83 deletions(-)
> 
> diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
> index 88e42b5..562fb91 100644
> --- a/src/conf/virnetworkobj.c
> +++ b/src/conf/virnetworkobj.c
> @@ -107,6 +107,103 @@ virNetworkObjEndAPI(virNetworkObjPtr *net)
>  }
>  
>  
> +virMacMapPtr
> +virNetworkObjGetMacMap(virNetworkObjPtr obj)
> +{
> +    return obj->macmap;
> +}
> +
> +
> +void
> +virNetworkObjSetMacMap(virNetworkObjPtr obj,
> +                       virMacMapPtr macmap)
> +{
> +    obj->macmap = macmap;
> +}
> +
> +
> +void
> +virNetworkObjUnrefMacMap(virNetworkObjPtr obj)
> +{
> +    if (!virObjectUnref(obj->macmap))
> +        obj->macmap = NULL;
> +}

You are just moving the code so it would be nice as a followup to fix
this function.  It seems kind of wrong to set obj->macmap to NULL only
if it was the last reference.  We should always set it to NULL because
the virNetworkObjDispose() would call virObjectUnref() again.  Currently
it doesn't hit any issue, but if someone gets the macmap by using
virNetworkObjGetMacMap() and creates its own reference, the
virNetworkObjDispose() would remove the reference and possible free the
macmap which would lead to crash.

> +
> +
> +char *
> +virNetworkObjMacMgrFileName(const char *dnsmasqStateDir,
> +                            const char *bridge)
> +{
> +    char *filename;
> +
> +    ignore_value(virAsprintf(&filename, "%s/%s.macs", dnsmasqStateDir, bridge));
> +
> +    return filename;
> +}

This function doesn't have anything in common with the virNetworkObj so
it should be moved into src/util/virmacmap.c since the output is used
only by virMacMap* functions from that module.

The rest of the patch seems to be correct.

Pavel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170724/fe2a5847/attachment-0001.sig>