[libvirt] [PATCH 3/6] virDomainFormatSchedDef: factor out subset code

[Martin Kletzander]

On Mon, Nov 07, 2016 at 10:01:14AM +0100, Martin Polednik wrote:
>The code within the function is too specific for priority attribute of
>RT schedulers. To allow addition of schedulers that group by different
>properties, we factor out the logic to calculate cpu subset. Instead
>of comparing by priority, the new code accepts comparator for the 2
>sched structs.
>---
> src/conf/domain_conf.c | 77 +++++++++++++++++++++++++++++++++++---------------
> 1 file changed, 55 insertions(+), 22 deletions(-)
>
>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index ca0fdfd..0ed755c 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -23058,6 +23058,47 @@ virDomainDefHasCapabilitiesFeatures(virDomainDefPtr def)
>     return false;
> }
>
>+static bool
>+virDomainSchedPriorityComparator(virDomainThreadSchedParamPtr baseSched,
>+                                 virDomainThreadSchedParamPtr sched)
>+{
>+    bool ret = false;
>+    ret = (baseSched->priority == sched->priority);
>+
>+    return ret;

Just return (baseSched->priority == sched->priority);  all else is not
needed here.

>+}
>+
>+static virDomainThreadSchedParamPtr
>+virDomainSchedSubsetCharacteristic(virDomainDefPtr def,
>+                                   virBitmapPtr schedMap,
>+                                   virBitmapPtr subsetMap,
>+                                   virDomainThreadSchedParamPtr (*func)(virDomainDefPtr, unsigned int),
>+                                   bool (*comparator)(virDomainThreadSchedParamPtr,
>+                                                      virDomainThreadSchedParamPtr))
>+{
>+    ssize_t nextidx;
>+    virDomainThreadSchedParamPtr sched;
>+    virDomainThreadSchedParamPtr baseSched = NULL;
>+
>+    virBitmapClearAll(subsetMap);
>+
>+    /* we need to find a subset of vCPUs with the given scheduler
>+        * that share the priority */
>+    nextidx = virBitmapNextSetBit(schedMap, -1);
>+    if (!(sched = func(def, nextidx)))
>+        return NULL;
>+
>+    baseSched = sched;
>+    ignore_value(virBitmapSetBit(subsetMap, nextidx));
>+
>+    while ((nextidx = virBitmapNextSetBit(schedMap, nextidx)) > -1) {
>+        sched = func(def, nextidx);
>+        if (sched && comparator(baseSched, sched))
>+            ignore_value(virBitmapSetBit(subsetMap, nextidx));
>+    }
>+
>+    return baseSched;
>+}
>
> /**
>  * virDomainFormatSchedDef:
>@@ -23082,8 +23123,9 @@ virDomainFormatSchedDef(virDomainDefPtr def,
>                         virBitmapPtr resourceMap)
> {
>     virBitmapPtr schedMap = NULL;
>-    virBitmapPtr prioMap = NULL;
>+    virBitmapPtr subsetMap = NULL;
>     virDomainThreadSchedParamPtr sched;
>+    virDomainThreadSchedParamPtr baseSched;
>     char *tmp = NULL;
>     ssize_t next;
>     size_t i;
>@@ -23098,7 +23140,7 @@ virDomainFormatSchedDef(virDomainDefPtr def,
>      */
>
>     if (!(schedMap = virBitmapNew(VIR_DOMAIN_CPUMASK_LEN)) ||
>-        !(prioMap = virBitmapNew(VIR_DOMAIN_CPUMASK_LEN)))
>+        !(subsetMap = virBitmapNew(VIR_DOMAIN_CPUMASK_LEN)))
>         goto cleanup;
>
>     for (i = VIR_PROC_POLICY_NONE + 1; i < VIR_PROC_POLICY_LAST; i++) {
>@@ -23117,9 +23159,8 @@ virDomainFormatSchedDef(virDomainDefPtr def,
>          * have them */
>         while (!virBitmapIsAllClear(schedMap)) {
>             virBitmapPtr currentMap = NULL;
>-            ssize_t nextprio;
>             bool hasPriority = false;
>-            int priority = 0;
>+            baseSched = NULL;
>
>             switch ((virProcessSchedPolicy) i) {
>             case VIR_PROC_POLICY_NONE:
>@@ -23132,25 +23173,17 @@ virDomainFormatSchedDef(virDomainDefPtr def,
>
>             case VIR_PROC_POLICY_FIFO:
>             case VIR_PROC_POLICY_RR:
>-                virBitmapClearAll(prioMap);
>                 hasPriority = true;
>
>-                /* we need to find a subset of vCPUs with the given scheduler
>-                 * that share the priority */
>-                nextprio = virBitmapNextSetBit(schedMap, -1);
>-                if (!(sched = func(def, nextprio)))
>+                baseSched = virDomainSchedSubsetCharacteristic(def,
>+                                                               schedMap,
>+                                                               subsetMap,
>+                                                               func,
>+                                                               virDomainSchedPriorityComparator);
>+                if (baseSched == NULL)
>                     goto cleanup;
>
>-                priority = sched->priority;
>-                ignore_value(virBitmapSetBit(prioMap, nextprio));
>-
>-                while ((nextprio = virBitmapNextSetBit(schedMap, nextprio)) > -1) {
>-                    sched = func(def, nextprio);
>-                    if (sched && sched->priority == priority)
>-                        ignore_value(virBitmapSetBit(prioMap, nextprio));
>-                }
>-
>-                currentMap = prioMap;
>+                currentMap = subsetMap;
>                 break;

This looks like it was factored out but with the combination of renaming
the variable with the same patch makes it weird to review.

>@@ -23164,8 +23197,8 @@ virDomainFormatSchedDef(virDomainDefPtr def,
>                               virProcessSchedPolicyTypeToString(i));
>             VIR_FREE(tmp);
>
>-            if (hasPriority)
>-                virBufferAsprintf(buf, " priority='%d'", priority);
>+            if (hasPriority && baseSched != NULL)
>+                virBufferAsprintf(buf, " priority='%d'", baseSched->priority);
>
>             virBufferAddLit(buf, "/>\n");
>
>@@ -23178,7 +23211,7 @@ virDomainFormatSchedDef(virDomainDefPtr def,
>
>  cleanup:
>     virBitmapFree(schedMap);
>-    virBitmapFree(prioMap);
>+    virBitmapFree(subsetMap);
>     return ret;
> }
>
>--
>2.8.1
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20161110/fffffdaf/attachment-0001.sig>