From: Vineeth Pillai <viremana@linux.microsoft.com>

Signed-off-by: Vineeth Pillai <viremana@linux.microsoft.com>
Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
---
 src/ch/ch_domain.c  |  30 +++++++++
 src/ch/ch_domain.h  |   7 ++-
 src/ch/ch_driver.c  | 145 ++++++++++++++++++++++++++++++++++++++++++++
 src/ch/ch_monitor.c |   2 +-
 4 files changed, 181 insertions(+), 3 deletions(-)

diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c
index 6f0cec8c6e..6fde7122f7 100644
--- a/src/ch/ch_domain.c
+++ b/src/ch/ch_domain.c
@@ -20,6 +20,7 @@
 
 #include <config.h>
 
+#include "datatypes.h"
 #include "ch_domain.h"
 #include "domain_driver.h"
 #include "viralloc.h"
@@ -416,3 +417,32 @@ virCHDomainGetMachineName(virDomainObj *vm)
 
     return ret;
 }
+
+/**
+ * virCHDomainObjFromDomain:
+ * @domain: Domain pointer that has to be looked up
+ *
+ * This function looks up @domain and returns the appropriate virDomainObjPtr
+ * that has to be released by calling virDomainObjEndAPI().
+ *
+ * Returns the domain object with incremented reference counter which is locked
+ * on success, NULL otherwise.
+ */
+virDomainObj *
+virCHDomainObjFromDomain(virDomainPtr domain)
+{
+    virDomainObj *vm;
+    virCHDriver *driver = domain->conn->privateData;
+    char uuidstr[VIR_UUID_STRING_BUFLEN];
+
+    vm = virDomainObjListFindByUUID(driver->domains, domain->uuid);
+    if (!vm) {
+        virUUIDFormat(domain->uuid, uuidstr);
+        virReportError(VIR_ERR_NO_DOMAIN,
+                       _("no domain with matching uuid '%s' (%s)"),
+                       uuidstr, domain->name);
+        return NULL;
+    }
+
+    return vm;
+}
diff --git a/src/ch/ch_domain.h b/src/ch/ch_domain.h
index cb94905b94..60a761ac84 100644
--- a/src/ch/ch_domain.h
+++ b/src/ch/ch_domain.h
@@ -97,5 +97,8 @@ virCHDomainGetVcpuPid(virDomainObj *vm,
 bool
 virCHDomainHasVcpuPids(virDomainObj *vm);
 
-char *
-virCHDomainGetMachineName(virDomainObj *vm);
+char
+*virCHDomainGetMachineName(virDomainObj *vm);
+
+virDomainObj
+*virCHDomainObjFromDomain(virDomain *domain);
diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
index 53e0872207..956189e83f 100644
--- a/src/ch/ch_driver.c
+++ b/src/ch/ch_driver.c
@@ -25,6 +25,7 @@
 #include "ch_driver.h"
 #include "ch_monitor.h"
 #include "ch_process.h"
+#include "domain_cgroup.h"
 #include "datatypes.h"
 #include "driver.h"
 #include "viraccessapicheck.h"
@@ -1129,6 +1130,148 @@ chDomainGetVcpus(virDomainPtr dom,
     return ret;
 }
 
+static int
+chDomainPinVcpuLive(virDomainObj *vm,
+                    virDomainDef *def,
+                    int vcpu,
+                    virCHDriver *driver,
+                    virCHDriverConfig *cfg,
+                    virBitmap *cpumap)
+{
+    g_autoptr(virBitmap) tmpmap = NULL;
+    g_autoptr(virCgroup) cgroup_vcpu = NULL;
+    virDomainVcpuDef *vcpuinfo;
+    virCHDomainObjPrivate *priv = vm->privateData;
+
+    g_autofree char *str = NULL;
+
+    if (!virCHDomainHasVcpuPids(vm)) {
+        virReportError(VIR_ERR_OPERATION_INVALID,
+                       "%s", _("cpu affinity is not supported"));
+        return -1;
+    }
+
+    if (!(vcpuinfo = virDomainDefGetVcpu(def, vcpu))) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("vcpu %d is out of range of live cpu count %d"),
+                       vcpu, virDomainDefGetVcpusMax(def));
+        return -1;
+    }
+
+    if (!(tmpmap = virBitmapNewCopy(cpumap)))
+        return -1;
+
+    if (!(str = virBitmapFormat(cpumap)))
+        return -1;
+
+    if (vcpuinfo->online) {
+        /* Configure the corresponding cpuset cgroup before set affinity. */
+        if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) {
+            if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_VCPU, vcpu,
+                                   false, &cgroup_vcpu) < 0)
+                return -1;
+            if (virDomainCgroupSetupCpusetCpus(cgroup_vcpu, cpumap) < 0)
+                return -1;
+        }
+
+        if (virProcessSetAffinity(virCHDomainGetVcpuPid(vm, vcpu), cpumap, false) < 0)
+            return -1;
+    }
+
+    virBitmapFree(vcpuinfo->cpumask);
+    vcpuinfo->cpumask = tmpmap;
+    tmpmap = NULL;
+
+    if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0)
+        return -1;
+
+    return 0;
+}
+
+
+static int
+chDomainPinVcpuFlags(virDomainPtr dom,
+                     unsigned int vcpu,
+                     unsigned char *cpumap,
+                     int maplen,
+                     unsigned int flags)
+{
+    virCHDriver *driver = dom->conn->privateData;
+    virDomainObj *vm;
+    virDomainDef *def;
+    virDomainDef *persistentDef;
+    int ret = -1;
+    virBitmap *pcpumap = NULL;
+    virDomainVcpuDef *vcpuinfo = NULL;
+    g_autoptr(virCHDriverConfig) cfg = NULL;
+
+    virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+                  VIR_DOMAIN_AFFECT_CONFIG, -1);
+
+    cfg = virCHDriverGetConfig(driver);
+
+    if (!(vm = virCHDomainObjFromDomain(dom)))
+        goto cleanup;
+
+    if (virDomainPinVcpuFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
+        goto cleanup;
+
+    if (virCHDomainObjBeginJob(vm, CH_JOB_MODIFY) < 0)
+        goto cleanup;
+
+    if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0)
+        goto endjob;
+
+    if (persistentDef &&
+        !(vcpuinfo = virDomainDefGetVcpu(persistentDef, vcpu))) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("vcpu %d is out of range of persistent cpu count %d"),
+                       vcpu, virDomainDefGetVcpus(persistentDef));
+        goto endjob;
+    }
+
+    if (!(pcpumap = virBitmapNewData(cpumap, maplen)))
+        goto endjob;
+
+    if (virBitmapIsAllClear(pcpumap)) {
+        virReportError(VIR_ERR_INVALID_ARG, "%s",
+                       _("Empty cpu list for pinning"));
+        goto endjob;
+    }
+
+    if (def &&
+        chDomainPinVcpuLive(vm, def, vcpu, driver, cfg, pcpumap) < 0)
+        goto endjob;
+
+    if (persistentDef) {
+        virBitmapFree(vcpuinfo->cpumask);
+        vcpuinfo->cpumask = pcpumap;
+        pcpumap = NULL;
+
+        goto endjob;
+    }
+
+    ret = 0;
+
+ endjob:
+    virCHDomainObjEndJob(vm);
+
+ cleanup:
+    virDomainObjEndAPI(&vm);
+    virBitmapFree(pcpumap);
+    return ret;
+}
+
+static int
+chDomainPinVcpu(virDomainPtr dom,
+                unsigned int vcpu,
+                unsigned char *cpumap,
+                int maplen)
+{
+    return chDomainPinVcpuFlags(dom, vcpu, cpumap, maplen,
+                                  VIR_DOMAIN_AFFECT_LIVE);
+}
+
 /* Function Tables */
 static virHypervisorDriver chHypervisorDriver = {
     .name = "CH",
@@ -1169,6 +1312,8 @@ static virHypervisorDriver chHypervisorDriver = {
     .domainGetVcpusFlags = chDomainGetVcpusFlags,           /* 8.0.0 */
     .domainGetMaxVcpus = chDomainGetMaxVcpus,               /* 8.0.0 */
     .domainGetVcpuPinInfo = chDomainGetVcpuPinInfo,         /* 8.0.0 */
+    .domainPinVcpu = chDomainPinVcpu,                       /* 8.1.0 */
+    .domainPinVcpuFlags = chDomainPinVcpuFlags,             /* 8.1.0 */
     .nodeGetCPUMap = chNodeGetCPUMap,                       /* 8.0.0 */
 };
 
diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
index d984bf9822..6c1d889a82 100644
--- a/src/ch/ch_monitor.c
+++ b/src/ch/ch_monitor.c
@@ -41,7 +41,7 @@ VIR_LOG_INIT("ch.ch_monitor");
 
 static virClass *virCHMonitorClass;
 static void virCHMonitorDispose(void *obj);
-static void virCHMonitorThreadInfoFree(virCHMonitor * mon);
+static void virCHMonitorThreadInfoFree(virCHMonitor *mon);
 
 static int virCHMonitorOnceInit(void)
 {
-- 
2.27.0

On 1/25/22 17:19, Praveen K Paladugu wrote:
> From: Vineeth Pillai <viremana@linux.microsoft.com>
> 
> Signed-off-by: Vineeth Pillai <viremana@linux.microsoft.com>
> Signed-off-by: Praveen K Paladugu <prapal@linux.microsoft.com>
> ---
>  src/ch/ch_domain.c  |  30 +++++++++
>  src/ch/ch_domain.h  |   7 ++-
>  src/ch/ch_driver.c  | 145 ++++++++++++++++++++++++++++++++++++++++++++
>  src/ch/ch_monitor.c |   2 +-
>  4 files changed, 181 insertions(+), 3 deletions(-)
> 
> diff --git a/src/ch/ch_domain.c b/src/ch/ch_domain.c
> index 6f0cec8c6e..6fde7122f7 100644
> --- a/src/ch/ch_domain.c
> +++ b/src/ch/ch_domain.c
> @@ -20,6 +20,7 @@
>  
>  #include <config.h>
>  
> +#include "datatypes.h"
>  #include "ch_domain.h"
>  #include "domain_driver.h"
>  #include "viralloc.h"
> @@ -416,3 +417,32 @@ virCHDomainGetMachineName(virDomainObj *vm)
>  
>      return ret;
>  }
> +
> +/**
> + * virCHDomainObjFromDomain:
> + * @domain: Domain pointer that has to be looked up
> + *
> + * This function looks up @domain and returns the appropriate virDomainObjPtr
> + * that has to be released by calling virDomainObjEndAPI().
> + *
> + * Returns the domain object with incremented reference counter which is locked
> + * on success, NULL otherwise.
> + */
> +virDomainObj *
> +virCHDomainObjFromDomain(virDomainPtr domain)
> +{
> +    virDomainObj *vm;
> +    virCHDriver *driver = domain->conn->privateData;
> +    char uuidstr[VIR_UUID_STRING_BUFLEN];
> +
> +    vm = virDomainObjListFindByUUID(driver->domains, domain->uuid);
> +    if (!vm) {
> +        virUUIDFormat(domain->uuid, uuidstr);
> +        virReportError(VIR_ERR_NO_DOMAIN,
> +                       _("no domain with matching uuid '%s' (%s)"),
> +                       uuidstr, domain->name);
> +        return NULL;
> +    }
> +
> +    return vm;
> +}

I'm not against moving this function, but:
1) it needs to be done in a separate commit
2) don't forget to remove the function from its original place (ch_driver.c)

> diff --git a/src/ch/ch_domain.h b/src/ch/ch_domain.h
> index cb94905b94..60a761ac84 100644
> --- a/src/ch/ch_domain.h
> +++ b/src/ch/ch_domain.h
> @@ -97,5 +97,8 @@ virCHDomainGetVcpuPid(virDomainObj *vm,
>  bool
>  virCHDomainHasVcpuPids(virDomainObj *vm);
>  
> -char *
> -virCHDomainGetMachineName(virDomainObj *vm);
> +char
> +*virCHDomainGetMachineName(virDomainObj *vm);
> +
> +virDomainObj
> +*virCHDomainObjFromDomain(virDomain *domain);
> diff --git a/src/ch/ch_driver.c b/src/ch/ch_driver.c
> index 53e0872207..956189e83f 100644
> --- a/src/ch/ch_driver.c
> +++ b/src/ch/ch_driver.c
> @@ -25,6 +25,7 @@
>  #include "ch_driver.h"
>  #include "ch_monitor.h"
>  #include "ch_process.h"
> +#include "domain_cgroup.h"
>  #include "datatypes.h"
>  #include "driver.h"
>  #include "viraccessapicheck.h"
> @@ -1129,6 +1130,148 @@ chDomainGetVcpus(virDomainPtr dom,
>      return ret;
>  }
>  
> +static int
> +chDomainPinVcpuLive(virDomainObj *vm,
> +                    virDomainDef *def,
> +                    int vcpu,
> +                    virCHDriver *driver,
> +                    virCHDriverConfig *cfg,
> +                    virBitmap *cpumap)
> +{
> +    g_autoptr(virBitmap) tmpmap = NULL;
> +    g_autoptr(virCgroup) cgroup_vcpu = NULL;
> +    virDomainVcpuDef *vcpuinfo;
> +    virCHDomainObjPrivate *priv = vm->privateData;
> +
> +    g_autofree char *str = NULL;
> +
> +    if (!virCHDomainHasVcpuPids(vm)) {
> +        virReportError(VIR_ERR_OPERATION_INVALID,
> +                       "%s", _("cpu affinity is not supported"));
> +        return -1;
> +    }
> +
> +    if (!(vcpuinfo = virDomainDefGetVcpu(def, vcpu))) {
> +        virReportError(VIR_ERR_INVALID_ARG,
> +                       _("vcpu %d is out of range of live cpu count %d"),
> +                       vcpu, virDomainDefGetVcpusMax(def));
> +        return -1;
> +    }
> +
> +    if (!(tmpmap = virBitmapNewCopy(cpumap)))
> +        return -1;
> +
> +    if (!(str = virBitmapFormat(cpumap)))
> +        return -1;
> +
> +    if (vcpuinfo->online) {
> +        /* Configure the corresponding cpuset cgroup before set affinity. */
> +        if (virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) {
> +            if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_VCPU, vcpu,
> +                                   false, &cgroup_vcpu) < 0)
> +                return -1;
> +            if (virDomainCgroupSetupCpusetCpus(cgroup_vcpu, cpumap) < 0)
> +                return -1;
> +        }
> +
> +        if (virProcessSetAffinity(virCHDomainGetVcpuPid(vm, vcpu), cpumap, false) < 0)
> +            return -1;
> +    }
> +
> +    virBitmapFree(vcpuinfo->cpumask);
> +    vcpuinfo->cpumask = tmpmap;
> +    tmpmap = NULL;

g_steal_pointer() in other words.

> +
> +    if (virDomainObjSave(vm, driver->xmlopt, cfg->stateDir) < 0)
> +        return -1;
> +
> +    return 0;
> +}
> +
> +
> +static int
> +chDomainPinVcpuFlags(virDomainPtr dom,
> +                     unsigned int vcpu,
> +                     unsigned char *cpumap,
> +                     int maplen,
> +                     unsigned int flags)
> +{
> +    virCHDriver *driver = dom->conn->privateData;
> +    virDomainObj *vm;
> +    virDomainDef *def;
> +    virDomainDef *persistentDef;
> +    int ret = -1;
> +    virBitmap *pcpumap = NULL;

g_autoptr(virBitmap)

> +    virDomainVcpuDef *vcpuinfo = NULL;
> +    g_autoptr(virCHDriverConfig) cfg = NULL;
> +
> +    virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
> +                  VIR_DOMAIN_AFFECT_CONFIG, -1);
> +
> +    cfg = virCHDriverGetConfig(driver);
> +
> +    if (!(vm = virCHDomainObjFromDomain(dom)))
> +        goto cleanup;
> +
> +    if (virDomainPinVcpuFlagsEnsureACL(dom->conn, vm->def, flags) < 0)
> +        goto cleanup;
> +
> +    if (virCHDomainObjBeginJob(vm, CH_JOB_MODIFY) < 0)
> +        goto cleanup;
> +
> +    if (virDomainObjGetDefs(vm, flags, &def, &persistentDef) < 0)
> +        goto endjob;
> +
> +    if (persistentDef &&
> +        !(vcpuinfo = virDomainDefGetVcpu(persistentDef, vcpu))) {
> +        virReportError(VIR_ERR_INVALID_ARG,
> +                       _("vcpu %d is out of range of persistent cpu count %d"),
> +                       vcpu, virDomainDefGetVcpus(persistentDef));
> +        goto endjob;
> +    }
> +
> +    if (!(pcpumap = virBitmapNewData(cpumap, maplen)))
> +        goto endjob;
> +
> +    if (virBitmapIsAllClear(pcpumap)) {
> +        virReportError(VIR_ERR_INVALID_ARG, "%s",
> +                       _("Empty cpu list for pinning"));
> +        goto endjob;
> +    }
> +
> +    if (def &&
> +        chDomainPinVcpuLive(vm, def, vcpu, driver, cfg, pcpumap) < 0)
> +        goto endjob;
> +
> +    if (persistentDef) {
> +        virBitmapFree(vcpuinfo->cpumask);
> +        vcpuinfo->cpumask = pcpumap;
> +        pcpumap = NULL;

g_steal_pointer()

> +
> +        goto endjob;
> +    }
> +
> +    ret = 0;
> +
> + endjob:
> +    virCHDomainObjEndJob(vm);
> +
> + cleanup:
> +    virDomainObjEndAPI(&vm);
> +    virBitmapFree(pcpumap);
> +    return ret;
> +}
> +
> +static int
> +chDomainPinVcpu(virDomainPtr dom,
> +                unsigned int vcpu,
> +                unsigned char *cpumap,
> +                int maplen)
> +{
> +    return chDomainPinVcpuFlags(dom, vcpu, cpumap, maplen,
> +                                  VIR_DOMAIN_AFFECT_LIVE);

misaligned

> +}
> +
>  /* Function Tables */
>  static virHypervisorDriver chHypervisorDriver = {
>      .name = "CH",
> @@ -1169,6 +1312,8 @@ static virHypervisorDriver chHypervisorDriver = {
>      .domainGetVcpusFlags = chDomainGetVcpusFlags,           /* 8.0.0 */
>      .domainGetMaxVcpus = chDomainGetMaxVcpus,               /* 8.0.0 */
>      .domainGetVcpuPinInfo = chDomainGetVcpuPinInfo,         /* 8.0.0 */
> +    .domainPinVcpu = chDomainPinVcpu,                       /* 8.1.0 */
> +    .domainPinVcpuFlags = chDomainPinVcpuFlags,             /* 8.1.0 */
>      .nodeGetCPUMap = chNodeGetCPUMap,                       /* 8.0.0 */
>  };
>  
> diff --git a/src/ch/ch_monitor.c b/src/ch/ch_monitor.c
> index d984bf9822..6c1d889a82 100644
> --- a/src/ch/ch_monitor.c
> +++ b/src/ch/ch_monitor.c
> @@ -41,7 +41,7 @@ VIR_LOG_INIT("ch.ch_monitor");
>  
>  static virClass *virCHMonitorClass;
>  static void virCHMonitorDispose(void *obj);
> -static void virCHMonitorThreadInfoFree(virCHMonitor * mon);
> +static void virCHMonitorThreadInfoFree(virCHMonitor *mon);

This does not belong here. If anything, it should have been done in the
previous patch that introduced this function.

Michal