On 1/25/22 17:19, Praveen K Paladugu wrote:
> From: Vineeth Pillai <viremana@xxxxxxxxxxxxxxxxxxx>
>
> Signed-off-by: Vineeth Pillai <viremana@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Praveen K Paladugu <prapal@xxxxxxxxxxxxxxxxxxx>
> ---
> src/ch/ch_conf.c | 2 +
> src/ch/ch_conf.h | 4 +-
> src/ch/ch_domain.c | 34 +++++
> src/ch/ch_domain.h | 11 +-
> src/ch/ch_monitor.c | 96 ++++++++++++++
> src/ch/ch_monitor.h | 54 +++++++-
> src/ch/ch_process.c | 308 ++++++++++++++++++++++++++++++++++++++++++--
> src/ch/ch_process.h | 3 +
> 8 files changed, 492 insertions(+), 20 deletions(-)
>
> diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c
> index a746d0f5fd..6f0cec8c6e 100644
> --- a/src/ch/ch_domain.c
> +++ b/src/ch/ch_domain.c
> @@ -319,6 +319,40 @@ chValidateDomainDeviceDef(const virDomainDeviceDef *dev,
> _("Serial can only be enabled for a PTY"));
> return -1;
> }
> + return 0;
> +}
> +
> +int
> +virCHDomainRefreshThreadInfo(virDomainObj *vm)
> +{
> + size_t maxvcpus = virDomainDefGetVcpusMax(vm->def);
> + virCHMonitorThreadInfo *info = NULL;
> + size_t nthreads, ncpus = 0;
We like one variable per line more, because it allows smaller diffs
should this area of code be ever changed.
> + size_t i;
> +
> + nthreads = virCHMonitorGetThreadInfo(virCHDomainGetMonitor(vm),
> + true, &info);
> +
> + for (i = 0; i < nthreads; i++) {
> + virCHDomainVcpuPrivate *vcpupriv;
> + virDomainVcpuDef *vcpu;
> + virCHMonitorCPUInfo *vcpuInfo;
> +
> + if (info[i].type != virCHThreadTypeVcpu)
> + continue;
> +
> + /* TODO: hotplug support */
> + vcpuInfo = &info[i].vcpuInfo;
> + vcpu = virDomainDefGetVcpu(vm->def, vcpuInfo->cpuid);
> + vcpupriv = CH_DOMAIN_VCPU_PRIVATE(vcpu);
> + vcpupriv->tid = vcpuInfo->tid;
> + ncpus++;
> + }
> +
> + /* TODO: Remove the warning when hotplug is implemented.*/
> + if (ncpus != maxvcpus)
> + VIR_WARN("Mismatch in the number of cpus, expected: %ld, actual: %ld",
> + maxvcpus, ncpus);
>
> return 0;
> }
> diff --git a/src/ch/ch_domain.h b/src/ch/ch_domain.h
> index 4d0b5479b8..cb94905b94 100644
> --- a/src/ch/ch_domain.h
> +++ b/src/ch/ch_domain.h
> @@ -53,11 +53,13 @@ typedef struct _virCHDomainObjPrivate virCHDomainObjPrivate;
> struct _virCHDomainObjPrivate {
> struct virCHDomainJobObj job;
>
> - virChrdevs *chrdevs;
> - virCHDriver *driver;
> - virCHMonitor *monitor;
> char *machineName;
> virBitmap *autoCpuset;
> + virBitmap *autoNodeset;
> + virCHDriver *driver;
> + virCHMonitor *monitor;
> + virCgroup *cgroup;
> + virChrdevs *chrdevs;
Looks like you can't make up your mind about the order. I remember
seeing the order of the members being changed over and over. Any reason
for that?
> };
>
> #define CH_DOMAIN_PRIVATE(vm) \
> @@ -87,7 +89,8 @@ void
> virCHDomainObjEndJob(virDomainObj *obj);
>
> int
> -virCHDomainRefreshVcpuInfo(virDomainObj *vm);
> +virCHDomainRefreshThreadInfo(virDomainObj *vm);
> +
> pid_t
> virCHDomainGetVcpuPid(virDomainObj *vm,
> unsigned int vcpuid);
> diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
> index a19f0c7e33..d984bf9822 100644
> --- a/src/ch/ch_monitor.c
> +++ b/src/ch/ch_monitor.c
> @@ -41,6 +41,7 @@ VIR_LOG_INIT("ch.ch_monitor");
>
> static virClass *virCHMonitorClass;
> static void virCHMonitorDispose(void *obj);
> +static void virCHMonitorThreadInfoFree(virCHMonitor * mon);
>
> static int virCHMonitorOnceInit(void)
> {
> @@ -578,6 +579,7 @@ static void virCHMonitorDispose(void *opaque)
> virCHMonitor *mon = opaque;
>
> VIR_DEBUG("mon=%p", mon);
> + virCHMonitorThreadInfoFree(mon);
> virObjectUnref(mon->vm);
> }
>
> @@ -743,6 +745,100 @@ virCHMonitorGet(virCHMonitor *mon, const char *endpoint, virJSONValue **response
> return ret;
> }
>
> +static void
> +virCHMonitorThreadInfoFree(virCHMonitor *mon)
> +{
> + mon->nthreads = 0;
> + if (mon->threads)
> + VIR_FREE(mon->threads);
Since VIR_FREE() is really just g_clear_pointer() there's no point in
checking the pointer. In fact, glib will do that for us.
> +}
> +
> +static size_t
> +virCHMonitorRefreshThreadInfo(virCHMonitor *mon)
> +{
> + virCHMonitorThreadInfo *info = NULL;
> + g_autofree pid_t *tids = NULL;
> + virDomainObj *vm = mon->vm;
> + size_t ntids = 0;
> + size_t i;
> +
> +
> + virCHMonitorThreadInfoFree(mon);
> + if (virProcessGetPids(vm->pid, &ntids, &tids) < 0) {
> + mon->threads = NULL;
No need to set to NULL. It was done by virCHMonitorThreadInfoFree() above.
> + return 0;
> + }
> +
> diff --git a/src/ch/ch_process.c b/src/ch/ch_process.c
> index 49976d769e..1a0730a4d1 100644
> --- a/src/ch/ch_process.c
> +++ b/src/ch/ch_process.c
> +/**
> + * virCHProcessSetupPid:
> + *
> + * This function sets resource properties (affinity, cgroups,
> + * scheduler) for any PID associated with a domain. It should be used
> + * to set up emulator PIDs as well as vCPU and I/O thread pids to
> + * ensure they are all handled the same way.
> + *
> + * Returns 0 on success, -1 on error.
> + */
> +static int
> +virCHProcessSetupPid(virDomainObj *vm,
> + pid_t pid,
> + virCgroupThreadName nameval,
> + int id,
> + virBitmap *cpumask,
> + unsigned long long period,
> + long long quota,
> + virDomainThreadSchedParam *sched)
> +{
> + virCHDomainObjPrivate *priv = vm->privateData;
> + virDomainNumatuneMemMode mem_mode;
> + virCgroup *cgroup = NULL;
g_autoptr(virCgroup)
> + virBitmap *use_cpumask = NULL;
> + virBitmap *affinity_cpumask = NULL;
> + g_autoptr(virBitmap) hostcpumap = NULL;
> + g_autofree char *mem_mask = NULL;
> + int ret = -1;
Michal
