Re: [libvirt PATCH v2 13/13] qemu_conf: Use automatic memory management

[Michal Prívozník <mprivozn@xxxxxxxxxx>]

On 3/16/22 23:10, Tim Wiederhake wrote:
> Signed-off-by: Tim Wiederhake <twiederh@xxxxxxxxxx>
> ---
>  src/qemu/qemu_conf.c | 70 ++++++++++++++++++--------------------------
>  1 file changed, 29 insertions(+), 41 deletions(-)
> 
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 81449b8b77..4b5f75b694 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -85,17 +85,6 @@ static int virQEMUConfigOnceInit(void)
>  VIR_ONCE_GLOBAL_INIT(virQEMUConfig);
>  
>  
> -static void
> -qemuDriverLock(virQEMUDriver *driver)
> -{
> -    virMutexLock(&driver->lock);
> -}
> -static void
> -qemuDriverUnlock(virQEMUDriver *driver)
> -{
> -    virMutexUnlock(&driver->lock);
> -}
> -
>  #ifndef DEFAULT_LOADER_NVRAM
>  # define DEFAULT_LOADER_NVRAM \
>      "/usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd:" \
> @@ -1279,11 +1268,9 @@ virQEMUDriverConfigSetDefaults(virQEMUDriverConfig *cfg)
>  
>  virQEMUDriverConfig *virQEMUDriverGetConfig(virQEMUDriver *driver)
>  {
> -    virQEMUDriverConfig *conf;
> -    qemuDriverLock(driver);
> -    conf = virObjectRef(driver->config);
> -    qemuDriverUnlock(driver);
> -    return conf;
> +    VIR_LOCK_GUARD lock = virLockGuardLock(&driver->lock);
> +
> +    return virObjectRef(driver->config);
>  }
>  
>  virDomainXMLOption *
> @@ -1303,16 +1290,13 @@ virQEMUDriverCreateXMLConf(virQEMUDriver *driver,
>  virCPUDef *
>  virQEMUDriverGetHostCPU(virQEMUDriver *driver)
>  {
> -    virCPUDef *hostcpu;
> -
> -    qemuDriverLock(driver);
> -
> -    if (!driver->hostcpu)
> -        driver->hostcpu = virCPUProbeHost(virArchFromHost());
> +    virCPUDef *hostcpu = NULL;
>  
> -    hostcpu = driver->hostcpu;
> -
> -    qemuDriverUnlock(driver);
> +    VIR_WITH_MUTEX_LOCK_GUARD(&driver->lock) {
> +        if (!driver->hostcpu)
> +            driver->hostcpu = virCPUProbeHost(virArchFromHost());
> +        hostcpu = driver->hostcpu;
> +    }
>  
>      if (hostcpu)
>          virCPUDefRef(hostcpu);
> @@ -1389,32 +1373,36 @@ virCaps *virQEMUDriverCreateCapabilities(virQEMUDriver *driver)
>   * Returns: a reference to a virCaps *instance or NULL
>   */
>  virCaps *virQEMUDriverGetCapabilities(virQEMUDriver *driver,
> -                                        bool refresh)
> +                                      bool refresh)
>  {

I'd rewrite this slightly different:

diff --git i/src/qemu/qemu_conf.c w/src/qemu/qemu_conf.c
index 4b5f75b694..c22cf79cbe 100644
--- i/src/qemu/qemu_conf.c
+++ w/src/qemu/qemu_conf.c
@@ -1375,9 +1375,6 @@ virCaps *virQEMUDriverCreateCapabilities(virQEMUDriver *driver)
 virCaps *virQEMUDriverGetCapabilities(virQEMUDriver *driver,
                                       bool refresh)
 {
-    bool force_refresh = false;
-    virCaps *ret = NULL;
-
     if (refresh) {
         virCaps *caps = NULL;
         if ((caps = virQEMUDriverCreateCapabilities(driver)) == NULL)
@@ -1386,21 +1383,15 @@ virCaps *virQEMUDriverGetCapabilities(virQEMUDriver *driver,
         VIR_WITH_MUTEX_LOCK_GUARD(&driver->lock) {
             virObjectUnref(driver->caps);
             driver->caps = caps;
-            ret = virObjectRef(driver->caps);
+            return virObjectRef(driver->caps);
         }
-
-        return ret;
     }
 
     VIR_WITH_MUTEX_LOCK_GUARD(&driver->lock) {
-        force_refresh = driver->caps == NULL || driver->caps->nguests == 0;
-        if (!force_refresh)
-            ret = virObjectRef(driver->caps);
+        if (driver->caps && driver->caps->nguests > 0)
+            return virObjectRef(driver->caps);
     }
 
-    if (!force_refresh)
-        return ret;
-
     VIR_DEBUG("Capabilities didn't detect any guests. Forcing a refresh.");
     return virQEMUDriverGetCapabilities(driver, true);
 }


Michal