[PATCH 2/5] ch_monitor: Correctly close and ref the virCHMonitor

Laine Stump laine at redhat.com
Sun Oct 3 19:43:52 UTC 2021


On 10/1/21 2:12 PM, William Douglas wrote:
> In virCHMontiorNew the monitor object is referenced an additional time
> incorrectly preventing it from being disposed of. Because the disposal
> wasn't being used, a bug in virCHMonitorClose that would incorrectly
> unref the domain object wasn't being seen. This change fixes both.

Although each is very small, the fixes should be in separate patches 
since they are separate and unconnected problems. If it's okay with you, 
I'll just split this patch and adjust the log comments (while still 
attributing to you) before pushing. Or if you'd rather, you can resend 
two separate patches for it, with log messages of your choice. Let me 
know which you prefer.

Otherwise:

Reviewed-by: Laine Stump <laine at redhat.com>

> 
> Signed-off-by: William Douglas <william.douglas at intel.com>
> ---
>   src/ch/ch_monitor.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
> index a1430f0e65..800457af41 100644
> --- a/src/ch/ch_monitor.c
> +++ b/src/ch/ch_monitor.c
> @@ -468,7 +468,7 @@ virCHMonitorNew(virDomainObj *vm, const char *socketdir)
>       if (!vm->def) {
>           virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                          _("VM is not defined"));
> -        return NULL;
> +        goto cleanup;
>       }
>   
>       /* prepare to launch Cloud-Hypervisor socket */
> @@ -502,12 +502,14 @@ virCHMonitorNew(virDomainObj *vm, const char *socketdir)
>       mon->handle = curl_easy_init();
>   
>       /* now has its own reference */
> -    virObjectRef(mon);
>       mon->vm = virObjectRef(vm);
>   
>       ret = mon;
> +    mon = NULL;
>   
>    cleanup:
> +    if (mon)
> +        virCHMonitorClose(mon);
>       virCommandFree(cmd);
>       return ret;
>   }
> @@ -542,7 +544,6 @@ void virCHMonitorClose(virCHMonitor *mon)
>           g_free(mon->socketpath);
>       }
>   
> -    virObjectUnref(mon->vm);
>       virObjectUnref(mon);
>   }
>   
> 




More information about the libvir-list mailing list