[libvirt] [PATCH v2 11/20] network: Introduce virNetworkObj{Get|Set}Autostart

[Michal Privoznik]

On 07/26/2017 05:05 PM, John Ferlan wrote:
> In preparation for privatizing the virNetworkObj structure, create
> accessors for the obj->autostart.
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/conf/virnetworkobj.c    | 15 +++++++++++++++
>  src/conf/virnetworkobj.h    |  9 ++++++++-
>  src/libvirt_private.syms    |  2 ++
>  src/network/bridge_driver.c | 20 ++++++++++----------
>  src/test/test_driver.c      |  5 +++--
>  5 files changed, 38 insertions(+), 13 deletions(-)
> 
> diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
> index 631f8cd..36d4bff 100644
> --- a/src/conf/virnetworkobj.c
> +++ b/src/conf/virnetworkobj.c
> @@ -129,6 +129,21 @@ virNetworkObjGetNewDef(virNetworkObjPtr obj)
>  }
>  
>  
> +int
> +virNetworkObjGetAutostart(virNetworkObjPtr obj)
> +{
> +    return obj->autostart;
> +}
> +
> +
> +void
> +virNetworkObjSetAutostart(virNetworkObjPtr obj,
> +                          int autostart)
> +{
> +    obj->autostart = autostart;
> +}
> +
> +
>  pid_t
>  virNetworkObjGetDnsmasqPid(virNetworkObjPtr obj)
>  {
> diff --git a/src/conf/virnetworkobj.h b/src/conf/virnetworkobj.h
> index 90ce0ca..a526d30 100644
> --- a/src/conf/virnetworkobj.h
> +++ b/src/conf/virnetworkobj.h
> @@ -32,7 +32,7 @@ struct _virNetworkObj {
>      pid_t dnsmasqPid;
>      pid_t radvdPid;
>      unsigned int active : 1;
> -    unsigned int autostart : 1;
> +    int autostart;

Since we are touching this does it make sense to convert this to bool?
Interestingly, you're doing that conversion for @active in the next patch.

>      unsigned int persistent : 1;
>  
>      virNetworkDefPtr def; /* The current definition */
> @@ -60,6 +60,13 @@ virNetworkObjSetDef(virNetworkObjPtr obj,
>  virNetworkDefPtr
>  virNetworkObjGetNewDef(virNetworkObjPtr obj);
>  
> +int
> +virNetworkObjGetAutostart(virNetworkObjPtr obj);
> +
> +void
> +virNetworkObjSetAutostart(virNetworkObjPtr obj,
> +                          int autostart);
> +
>  virMacMapPtr
>  virNetworkObjGetMacMap(virNetworkObjPtr obj);
>  
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 84e3eb7..8a3284f 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -944,6 +944,7 @@ virNetworkObjFindByName;
>  virNetworkObjFindByNameLocked;
>  virNetworkObjFindByUUID;
>  virNetworkObjFindByUUIDLocked;
> +virNetworkObjGetAutostart;
>  virNetworkObjGetClassIdMap;
>  virNetworkObjGetDef;
>  virNetworkObjGetDnsmasqPid;
> @@ -966,6 +967,7 @@ virNetworkObjNew;
>  virNetworkObjRemoveInactive;
>  virNetworkObjReplacePersistentDef;
>  virNetworkObjSaveStatus;
> +virNetworkObjSetAutostart;
>  virNetworkObjSetDef;
>  virNetworkObjSetDefTransient;
>  virNetworkObjSetDnsmasqPid;
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index b4fbfc5..eb814d3 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -525,7 +525,7 @@ networkAutostartConfig(virNetworkObjPtr obj,
>      int ret = -1;
>  
>      virObjectLock(obj);
> -    if (obj->autostart &&
> +    if (virNetworkObjGetAutostart(obj) &&
>          !virNetworkObjIsActive(obj) &&
>          networkStartNetwork(driver, obj) < 0)
>          goto cleanup;
> @@ -3963,7 +3963,7 @@ networkGetAutostart(virNetworkPtr net,
>      if (virNetworkGetAutostartEnsureACL(net->conn, virNetworkObjGetDef(obj)) < 0)
>          goto cleanup;
>  
> -    *autostart = obj->autostart;
> +    *autostart = virNetworkObjGetAutostart(obj);
>      ret = 0;
>  
>   cleanup:
> @@ -3974,12 +3974,13 @@ networkGetAutostart(virNetworkPtr net,
>  
>  static int
>  networkSetAutostart(virNetworkPtr net,
> -                    int autostart)
> +                    int new_autostart)
>  {
>      virNetworkDriverStatePtr driver = networkGetDriver();
>      virNetworkObjPtr obj;
>      virNetworkDefPtr def;
>      char *configFile = NULL, *autostartLink = NULL;
> +    int cur_autostart;
>      int ret = -1;
>  
>      if (!(obj = networkObjFromNetwork(net)))
> @@ -3995,9 +3996,9 @@ networkSetAutostart(virNetworkPtr net,
>          goto cleanup;
>      }
>  
> -    autostart = (autostart != 0);
> -
> -    if (obj->autostart != autostart) {
> +    new_autostart = (new_autostart != 0);
> +    cur_autostart = virNetworkObjGetAutostart(obj);
> +    if (cur_autostart != new_autostart) {
>          if ((configFile = virNetworkConfigFile(driver->networkConfigDir,
>                                                 def->name)) == NULL)
>              goto cleanup;
> @@ -4005,7 +4006,7 @@ networkSetAutostart(virNetworkPtr net,
>                                                    def->name)) == NULL)
>              goto cleanup;
>  
> -        if (autostart) {
> +        if (new_autostart) {
>              if (virFileMakePath(driver->networkAutostartDir) < 0) {
>                  virReportSystemError(errno,
>                                       _("cannot create autostart directory '%s'"),
> @@ -4028,13 +4029,12 @@ networkSetAutostart(virNetworkPtr net,
>              }
>          }
>  
> -        obj->autostart = autostart;
> +        virNetworkObjSetAutostart(obj, new_autostart);
>      }
> +
>      ret = 0;
>  
>   cleanup:
> -    VIR_FREE(configFile);
> -    VIR_FREE(autostartLink);

This doesn't feel right. @configFile and @autostartLink are leaked.

>      virNetworkObjEndAPI(&obj);
>      return ret;
>  }

Michal