[libvirt PATCH v5 6/7] ch_process: Setup emulator and iothread settings

[Michal Prívozník]

On 1/25/22 17:19, Praveen K Paladugu wrote:
> using virCHProcessSetupPid
> 
> Signed-off-by: Praveen K Paladugu <prapal at linux.microsoft.com>
> ---
>  src/ch/ch_monitor.c | 60 +++++++++++++++++++++++++++++++++++
>  src/ch/ch_monitor.h |  2 ++
>  src/ch/ch_process.c | 77 ++++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 138 insertions(+), 1 deletion(-)
> 
> diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
> index 6c1d889a82..2a7cffb696 100644
> --- a/src/ch/ch_monitor.c
> +++ b/src/ch/ch_monitor.c
> @@ -930,3 +930,63 @@ virCHMonitorGetInfo(virCHMonitor *mon, virJSONValue **info)
>  {
>      return virCHMonitorGet(mon, URL_VM_INFO, info);
>  }
> +
> +/**
> + * virCHMonitorGetIOThreads:
> + * @mon: Pointer to the monitor
> + * @iothreads: Location to return array of IOThreadInfo data
> + *
> + * Retrieve the list of iothreads defined/running for the machine
> + *
> + * Returns count of IOThreadInfo structures on success
> + *        -1 on error.
> + */
> +int virCHMonitorGetIOThreads(virCHMonitor *mon,
> +                             virDomainIOThreadInfo ***iothreads)
> +{
> +    size_t nthreads = 0, niothreads = 0;
> +    int thd_index;
> +    virDomainIOThreadInfo **iothreadinfolist = NULL, *iothreadinfo = NULL;
> +
> +    *iothreads = NULL;
> +    nthreads = virCHMonitorRefreshThreadInfo(mon);
> +
> +    iothreadinfolist = g_new0(virDomainIOThreadInfo*, nthreads);
> +
> +    for (thd_index = 0; thd_index < nthreads; thd_index++) {
> +        virBitmap *map = NULL;
> +        if (mon->threads[thd_index].type == virCHThreadTypeIO) {
> +            iothreadinfo = g_new0(virDomainIOThreadInfo, 1);
> +
> +            iothreadinfo->iothread_id = mon->threads[thd_index].ioInfo.tid;
> +
> +            if (!(map = virProcessGetAffinity(iothreadinfo->iothread_id)))
> +                goto cleanup;
> +
> +            if (virBitmapToData(map, &(iothreadinfo->cpumap),
> +                                &(iothreadinfo->cpumaplen)) < 0) {
> +                virBitmapFree(map);
> +                goto cleanup;
> +            }
> +            virBitmapFree(map);
> +            /* Append to iothreadinfolist */
> +            iothreadinfolist[niothreads] = iothreadinfo;
> +            niothreads++;
> +        }
> +    }
> +    VIR_DELETE_ELEMENT_INPLACE(iothreadinfolist,
> +                                       niothreads, nthreads);

I'm sorry, but what are you trying to say here? It looks like you're
trying to turn @iothreadinfolist into a null terminated array. Well, for
that all you need to to pass nthreads + 1 into g_new0() above.

> +    *iothreads = iothreadinfolist;
> +    VIR_DEBUG("niothreads = %ld", niothreads);
> +    return niothreads;
> +
> +    cleanup:

This needs to be named 'error' because the control gets here only in
error cases. The label is not used in successful path.

> +        if (iothreadinfolist) {
> +            for (thd_index = 0; thd_index < niothreads; thd_index++)
> +                VIR_FREE(iothreadinfolist[thd_index]);

Life's not that simple. The argument is type of virDomainIOThreadInfo*
and as such might have ->cpumap member allocated. You need to call
virDomainIOThreadInfoFree() instead of plain VIR_FREE().

> +            VIR_FREE(iothreadinfolist);

This one is okay, because this is just an array of pointers.

> +        }
> +        if (iothreadinfo)
> +            VIR_FREE(iothreadinfo);

And again, no need for the if() and plain VIR_FREE() is not enough.

> +        return -1;
> +}

Michal