Re: [PATCH 11/12] libxl: Remove unnecessary driver locking

[Jim Fehlig <jfehlig@xxxxxxxx>]

Jim Fehlig wrote:
> Now that most fields of libxlDriverPrivate struct are immutable
> or self-locking, there is no need to acquire the driver lock in
> much of the libxl driver.
>
> Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
> ---
>  src/libxl/libxl_conf.c   |   7 +-
>  src/libxl/libxl_driver.c | 217 +++++++++--------------------------------------
>  2 files changed, 46 insertions(+), 178 deletions(-)
>
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 19fd8a6..34f6bc1 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -1110,7 +1110,12 @@ error:
>  libxlDriverConfigPtr
>  libxlDriverConfigGet(libxlDriverPrivatePtr driver)
>  {
> -    return virObjectRef(driver->config);
> +    libxlDriverConfigPtr cfg;
> +
> +    libxlDriverLock(driver);
> +    cfg = virObjectRef(driver->config);
> +    libxlDriverUnlock(driver);
> +    return cfg;
>  }
>  
>  virCapsPtr
> diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
> index 8ece4c9..22bd26f 100644
> --- a/src/libxl/libxl_driver.c
> +++ b/src/libxl/libxl_driver.c
> @@ -77,7 +77,6 @@ static int
>  libxlVmStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
>               bool start_paused, int restore_fd);
>  
> -/* driver must be locked before calling */
>  static void
>  libxlDomainEventQueue(libxlDriverPrivatePtr driver, virDomainEventPtr event)
>  {
> @@ -156,17 +155,21 @@ libxlDomainManagedSavePath(libxlDriverPrivatePtr driver, virDomainObjPtr vm) {
>      return ret;
>  }
>  
> -/* This internal function expects the driver lock to already be held on
> - * entry. */
> -static int ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4)
> -libxlSaveImageOpen(libxlDriverPrivatePtr driver, const char *from,
> -                     virDomainDefPtr *ret_def, libxlSavefileHeaderPtr ret_hdr)
> +/*
> + * This internal function expects the driver lock to already be held on
> + * entry.
> + */
> +static int ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5)
> +libxlSaveImageOpen(libxlDriverPrivatePtr driver,
> +                   libxlDriverConfigPtr cfg,
> +                   const char *from,
> +                   virDomainDefPtr *ret_def,
> +                   libxlSavefileHeaderPtr ret_hdr)
>  {
>      int fd;
>      virDomainDefPtr def = NULL;
>      libxlSavefileHeader hdr;
>      char *xml = NULL;
> -    libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
>   

While rebasing this patch after making the changes to 7 suggested by
Michal, I realized that unref'ing the cfg object also needs to be
removed since it is now passed to this function and unref'ed by its
callers.  Squashing in the following hunks before pushing

@@ -212,7 +215,6 @@ libxlSaveImageOpen(libxlDriverPrivatePtr driver,
const char *from,
         goto error;
 
     VIR_FREE(xml);
-    virObjectUnref(cfg);
 
     *ret_def = def;
     *ret_hdr = hdr;
@@ -221,7 +223,6 @@ libxlSaveImageOpen(libxlDriverPrivatePtr driver,
const char *from,
 
 error:
     VIR_FREE(xml);
-    virObjectUnref(cfg);
     virDomainDefFree(def);
     VIR_FORCE_CLOSE(fd);
     return -1;

Regards,
Jim

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list