[libvirt] [PATCH resend V10 02/12] Resctrl: expose cache information to capabilities

Eli Qiao posted 12 patches 8 years, 4 months ago
[libvirt] [PATCH resend V10 02/12] Resctrl: expose cache information to capabilities
Posted by Eli Qiao 8 years, 4 months ago
This patch expose cache information to host's capabilites xml.

For l3 cache allocation
<cache>
  <bank id='0' type='l3' size='56320' unit='KiB' cpus='0-21,44-65'>
    <control min='2816' reserved='2816' unit='KiB' scope='L3'/>
  </bank>
  <bank id='1' type='l3' size='56320' unit='KiB' cpus='22-43,66-87'>
    <control min='2816' reserved='2816' unit='KiB' scope='L3'/>
  </bank>
</cache>

For l3 cache allocation supported cdp(seperate data/code):
<cache>
  <bank id='0' type='l3' size='56320' unit='KiB' cpus='0-21,44-65'>
    <control min='2816' reserved='2816' unit='KiB' scope='L3DATA'/>
    <control min='2816' reserved='2816' unit='KiB' scope='L3CODE'/>
  </bank>
  <bank id='1' type='l3' size='56320' unit='KiB' cpus='22-43,66-87'>
    <control min='2816' reserved='2816' unit='KiB' scope='L3DATA'/>
    <control min='2816' reserved='2816' unit='KiB' scope='L3CODE'/>
  </bank>
</cache>

RFC on mailing list.
https://www.redhat.com/archives/libvir-list/2017-January/msg00644.html

Signed-off-by: Eli Qiao <liyong.qiao@intel.com>
---
 src/conf/capabilities.c      | 56 ++++++++++++++++++++++++++++++++++++++
 src/conf/capabilities.h      | 23 ++++++++++++++++
 src/libvirt_private.syms     |  3 +++
 src/nodeinfo.c               | 64 ++++++++++++++++++++++++++++++++++++++++++++
 src/nodeinfo.h               |  1 +
 src/qemu/qemu_capabilities.c |  8 ++++++
 src/qemu/qemu_driver.c       |  4 +++
 7 files changed, 159 insertions(+)

diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
index 9ab343b..23e21e4 100644
--- a/src/conf/capabilities.c
+++ b/src/conf/capabilities.c
@@ -198,6 +198,18 @@ virCapabilitiesClearSecModel(virCapsHostSecModelPtr secmodel)
 }
 
 static void
+virCapabilitiesClearCacheBank(virCapsHostCacheBankPtr cachebank)
+{
+    size_t i;
+    for (i = 0; i < cachebank->ncontrol; i++)
+        VIR_FREE(cachebank->control[i].scope);
+
+    VIR_FREE(cachebank->type);
+    VIR_FREE(cachebank->cpus);
+}
+
+
+static void
 virCapabilitiesDispose(void *object)
 {
     virCapsPtr caps = object;
@@ -221,6 +233,10 @@ virCapabilitiesDispose(void *object)
         virCapabilitiesClearSecModel(&caps->host.secModels[i]);
     VIR_FREE(caps->host.secModels);
 
+    for (i = 0; i < caps->host.ncachebank; i++)
+        virCapabilitiesClearCacheBank(caps->host.cachebank[i]);
+    VIR_FREE(caps->host.cachebank);
+
     VIR_FREE(caps->host.netprefix);
     VIR_FREE(caps->host.pagesSize);
     virCPUDefFree(caps->host.cpu);
@@ -844,6 +860,41 @@ virCapabilitiesFormatNUMATopology(virBufferPtr buf,
     return 0;
 }
 
+static int
+virCapabilitiesFormatCache(virBufferPtr buf,
+                           size_t ncachebank,
+                           virCapsHostCacheBankPtr *cachebank)
+{
+    size_t i;
+    size_t j;
+
+    virBufferAddLit(buf, "<cache>\n");
+    virBufferAdjustIndent(buf, 2);
+
+    for (i = 0; i < ncachebank; i++) {
+        virBufferAsprintf(buf,
+                          "<bank id='%u' type='%s' size='%llu' unit='KiB' cpus='%s'>\n",
+                          cachebank[i]->id,
+                          cachebank[i]->type,
+                          cachebank[i]->size,
+                          cachebank[i]->cpus);
+
+        virBufferAdjustIndent(buf, 2);
+        for (j = 0; j < cachebank[i]->ncontrol; j++) {
+            virBufferAsprintf(buf,
+                              "<control min='%llu' reserved='%llu' unit='KiB' scope='%s'/>\n",
+                              cachebank[i]->control[j].min,
+                              cachebank[i]->control[j].reserved,
+                              cachebank[i]->control[j].scope);
+        }
+        virBufferAdjustIndent(buf, -2);
+        virBufferAddLit(buf, "</bank>\n");
+    }
+    virBufferAdjustIndent(buf, -2);
+    virBufferAddLit(buf, "</cache>\n");
+    return 0;
+}
+
 /**
  * virCapabilitiesFormatXML:
  * @caps: capabilities to format
@@ -931,6 +982,11 @@ virCapabilitiesFormatXML(virCapsPtr caps)
         virBufferAddLit(&buf, "</migration_features>\n");
     }
 
+    if (caps->host.ncachebank &&
+            virCapabilitiesFormatCache(&buf, caps->host.ncachebank,
+                                       caps->host.cachebank) < 0)
+        return NULL;
+
     if (caps->host.netprefix)
         virBufferAsprintf(&buf, "<netprefix>%s</netprefix>\n",
                           caps->host.netprefix);
diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
index cfdc34a..b446de5 100644
--- a/src/conf/capabilities.h
+++ b/src/conf/capabilities.h
@@ -138,6 +138,25 @@ struct _virCapsHostSecModel {
     virCapsHostSecModelLabelPtr labels;
 };
 
+typedef struct _virCapsHostCacheControl virCapsHostCacheControl;
+typedef virCapsHostCacheControl *virCapsHostCacheControlPtr;
+struct _virCapsHostCacheControl {
+    unsigned long long min;
+    unsigned long long reserved;
+    char* scope;
+};
+
+typedef struct _virCapsHostCacheBank virCapsHostCacheBank;
+typedef virCapsHostCacheBank *virCapsHostCacheBankPtr;
+struct _virCapsHostCacheBank {
+    unsigned int id;
+    char* type;
+    char* cpus;
+    unsigned long long size;
+    size_t ncontrol;
+    virCapsHostCacheControlPtr control;
+};
+
 typedef struct _virCapsHost virCapsHost;
 typedef virCapsHost *virCapsHostPtr;
 struct _virCapsHost {
@@ -160,6 +179,10 @@ struct _virCapsHost {
     size_t nsecModels;
     virCapsHostSecModelPtr secModels;
 
+    size_t ncachebank;
+    size_t ncachebank_max;
+    virCapsHostCacheBankPtr *cachebank;
+
     char *netprefix;
     virCPUDefPtr cpu;
     int nPagesSize;             /* size of pagesSize array */
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index bb7c3ad..b8445ef 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1125,6 +1125,7 @@ virLogManagerNew;
 # nodeinfo.h
 nodeCapsInitNUMA;
 nodeGetInfo;
+virCapsInitCache;
 virHostCPUGetCount;
 virHostCPUGetKVMMaxVCPUs;
 virHostCPUGetMap;
@@ -2322,8 +2323,10 @@ virRandomInt;
 
 # util/virresctrl.h
 virResCtrlAvailable;
+virResCtrlGet;
 virResCtrlInit;
 
+
 # util/virrotatingfile.h
 virRotatingFileReaderConsume;
 virRotatingFileReaderFree;
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index f2ded02..a001cd0 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -48,6 +48,7 @@
 #include "virstring.h"
 #include "virnuma.h"
 #include "virlog.h"
+#include "virresctrl.h"
 
 #define VIR_FROM_THIS VIR_FROM_NONE
 
@@ -416,3 +417,66 @@ nodeCapsInitNUMA(virCapsPtr caps)
     VIR_FREE(pageinfo);
     return ret;
 }
+
+int
+virCapsInitCache(virCapsPtr caps)
+{
+    size_t i, j;
+    virResCtrlPtr resctrl;
+    virCapsHostCacheBankPtr bank;
+
+    for (i = 0; i < VIR_RDT_RESOURCE_LAST; i++) {
+        /* L3DATA and L3CODE share L3 resources */
+        if (i == VIR_RDT_RESOURCE_L3CODE)
+            continue;
+
+        resctrl = virResCtrlGet(i);
+
+        if (resctrl->enabled) {
+            for (j = 0; j < resctrl->num_banks; j++) {
+                if (VIR_RESIZE_N(caps->host.cachebank, caps->host.ncachebank_max,
+                                caps->host.ncachebank, 1) < 0)
+                    return -1;
+
+                if (VIR_ALLOC(bank) < 0)
+                    return -1;
+
+                bank->id = resctrl->cache_banks[j].host_id;
+                if (VIR_STRDUP(bank->type, resctrl->cache_level) < 0)
+                    goto err;
+                if (VIR_STRDUP(bank->cpus, virBitmapFormat(resctrl->cache_banks[j].cpu_mask)) < 0)
+                    goto err;
+                bank->size = resctrl->cache_banks[j].cache_size;
+                /*L3DATA and L3CODE shares L3 cache resources, so fill them to the control element*/
+                if (i == VIR_RDT_RESOURCE_L3DATA) {
+                    if (VIR_EXPAND_N(bank->control, bank->ncontrol, 2) < 0)
+                        goto err;
+
+                    bank->control[0].min = virResCtrlGet(VIR_RDT_RESOURCE_L3DATA)->cache_banks[j].cache_min;
+                    bank->control[0].reserved = bank->control[0].min * (virResCtrlGet(VIR_RDT_RESOURCE_L3DATA)->min_cbm_bits);
+                    if (VIR_STRDUP(bank->control[0].scope,
+                                  virResCtrlGet(VIR_RDT_RESOURCE_L3DATA)->name) < 0)
+                        goto err;
+
+                    bank->control[1].min = virResCtrlGet(VIR_RDT_RESOURCE_L3CODE)->cache_banks[j].cache_min;
+                    bank->control[1].reserved = bank->control[1].min * (virResCtrlGet(VIR_RDT_RESOURCE_L3CODE)->min_cbm_bits);
+                    if (VIR_STRDUP(bank->control[1].scope,
+                                   virResCtrlGet(VIR_RDT_RESOURCE_L3CODE)->name) < 0)
+                        goto err;
+                } else {
+                    if (VIR_EXPAND_N(bank->control, bank->ncontrol, 1) < 0)
+                        goto err;
+                    bank->control[0].min = resctrl->cache_banks[j].cache_min;
+                    bank->control[0].reserved = bank->control[0].min * resctrl->min_cbm_bits;
+                    if (VIR_STRDUP(bank->control[0].scope, resctrl->name) < 0)
+                        goto err;
+                }
+                caps->host.cachebank[caps->host.ncachebank++] = bank;
+            }
+        }
+    }
+    return 0;
+ err:
+    VIR_FREE(bank);
+    return -1;
+}
diff --git a/src/nodeinfo.h b/src/nodeinfo.h
index 3c4dc46..5eb0f83 100644
--- a/src/nodeinfo.h
+++ b/src/nodeinfo.h
@@ -28,5 +28,6 @@
 
 int nodeGetInfo(virNodeInfoPtr nodeinfo);
 int nodeCapsInitNUMA(virCapsPtr caps);
+int virCapsInitCache(virCapsPtr caps);
 
 #endif /* __VIR_NODEINFO_H__*/
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 359a0d8..a0d7254 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -1100,6 +1100,11 @@ virQEMUCapsInitCPU(virCapsPtr caps,
     goto cleanup;
 }
 
+static int
+virQEMUCapsInitCache(virCapsPtr caps)
+{
+    return virCapsInitCache(caps);
+}
 
 static int
 virQEMUCapsInitPages(virCapsPtr caps)
@@ -1146,6 +1151,9 @@ virCapsPtr virQEMUCapsInit(virQEMUCapsCachePtr cache)
     if (virQEMUCapsInitCPU(caps, hostarch) < 0)
         VIR_WARN("Failed to get host CPU");
 
+    if (virQEMUCapsInitCache(caps) < 0)
+        VIR_WARN("Failed to get host cache");
+
     /* Add the power management features of the host */
     if (virNodeSuspendGetTargetMask(&caps->host.powerMgmt) < 0)
         VIR_WARN("Failed to get host power management capabilities");
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 77d8175..3bfb4ec 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -105,6 +105,7 @@
 #include "vircgroup.h"
 #include "virperf.h"
 #include "virnuma.h"
+#include "virresctrl.h"
 #include "dirname.h"
 #include "network/bridge_driver.h"
 
@@ -849,6 +850,9 @@ qemuStateInitialize(bool privileged,
         run_gid = cfg->group;
     }
 
+    if (virResCtrlAvailable() && virResCtrlInit() < 0)
+        VIR_WARN("Faild to initialize resource control.");
+
     qemu_driver->qemuCapsCache = virQEMUCapsCacheNew(cfg->libDir,
                                                      cfg->cacheDir,
                                                      run_uid,
-- 
1.9.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH resend V10 02/12] Resctrl: expose cache information to capabilities
Posted by Martin Kletzander 8 years, 3 months ago
On Mon, Mar 06, 2017 at 06:06:31PM +0800, Eli Qiao wrote:
>This patch expose cache information to host's capabilites xml.
>
>For l3 cache allocation
><cache>
>  <bank id='0' type='l3' size='56320' unit='KiB' cpus='0-21,44-65'>
>    <control min='2816' reserved='2816' unit='KiB' scope='L3'/>
>  </bank>
>  <bank id='1' type='l3' size='56320' unit='KiB' cpus='22-43,66-87'>
>    <control min='2816' reserved='2816' unit='KiB' scope='L3'/>
>  </bank>
></cache>
>
>For l3 cache allocation supported cdp(seperate data/code):
><cache>
>  <bank id='0' type='l3' size='56320' unit='KiB' cpus='0-21,44-65'>
>    <control min='2816' reserved='2816' unit='KiB' scope='L3DATA'/>
>    <control min='2816' reserved='2816' unit='KiB' scope='L3CODE'/>

I know this was discussed before, but why having a vector value in a
single attribute?  Why don't we split it to two scalars?  It will also
be SOO much more readable and close to what other tools expose, e.g.:

<control scope="data"/>
<control scope="instruction"/>

or

<control scope="both"/>

I also skipped the 'l3' because that is already in the <bank/> and will
never be different, right?

>  </bank>
>  <bank id='1' type='l3' size='56320' unit='KiB' cpus='22-43,66-87'>
>    <control min='2816' reserved='2816' unit='KiB' scope='L3DATA'/>
>    <control min='2816' reserved='2816' unit='KiB' scope='L3CODE'/>
>  </bank>
></cache>
>
>RFC on mailing list.
>https://www.redhat.com/archives/libvir-list/2017-January/msg00644.html
>
>Signed-off-by: Eli Qiao <liyong.qiao@intel.com>
>---
> src/conf/capabilities.c      | 56 ++++++++++++++++++++++++++++++++++++++
> src/conf/capabilities.h      | 23 ++++++++++++++++
> src/libvirt_private.syms     |  3 +++
> src/nodeinfo.c               | 64 ++++++++++++++++++++++++++++++++++++++++++++
> src/nodeinfo.h               |  1 +
> src/qemu/qemu_capabilities.c |  8 ++++++
> src/qemu/qemu_driver.c       |  4 +++
> 7 files changed, 159 insertions(+)
>
>diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
>index 9ab343b..23e21e4 100644
>--- a/src/conf/capabilities.c
>+++ b/src/conf/capabilities.c
>@@ -198,6 +198,18 @@ virCapabilitiesClearSecModel(virCapsHostSecModelPtr secmodel)
> }
>
> static void
>+virCapabilitiesClearCacheBank(virCapsHostCacheBankPtr cachebank)
>+{
>+    size_t i;
>+    for (i = 0; i < cachebank->ncontrol; i++)
>+        VIR_FREE(cachebank->control[i].scope);
>+
>+    VIR_FREE(cachebank->type);
>+    VIR_FREE(cachebank->cpus);
>+}
>+
>+
>+static void
> virCapabilitiesDispose(void *object)
> {
>     virCapsPtr caps = object;
>@@ -221,6 +233,10 @@ virCapabilitiesDispose(void *object)
>         virCapabilitiesClearSecModel(&caps->host.secModels[i]);
>     VIR_FREE(caps->host.secModels);
>
>+    for (i = 0; i < caps->host.ncachebank; i++)
>+        virCapabilitiesClearCacheBank(caps->host.cachebank[i]);
>+    VIR_FREE(caps->host.cachebank);
>+
>     VIR_FREE(caps->host.netprefix);
>     VIR_FREE(caps->host.pagesSize);
>     virCPUDefFree(caps->host.cpu);
>@@ -844,6 +860,41 @@ virCapabilitiesFormatNUMATopology(virBufferPtr buf,
>     return 0;
> }
>
>+static int
>+virCapabilitiesFormatCache(virBufferPtr buf,
>+                           size_t ncachebank,
>+                           virCapsHostCacheBankPtr *cachebank)
>+{
>+    size_t i;
>+    size_t j;
>+
>+    virBufferAddLit(buf, "<cache>\n");
>+    virBufferAdjustIndent(buf, 2);
>+
>+    for (i = 0; i < ncachebank; i++) {
>+        virBufferAsprintf(buf,
>+                          "<bank id='%u' type='%s' size='%llu' unit='KiB' cpus='%s'>\n",
>+                          cachebank[i]->id,
>+                          cachebank[i]->type,
>+                          cachebank[i]->size,
>+                          cachebank[i]->cpus);
>+
>+        virBufferAdjustIndent(buf, 2);
>+        for (j = 0; j < cachebank[i]->ncontrol; j++) {
>+            virBufferAsprintf(buf,
>+                              "<control min='%llu' reserved='%llu' unit='KiB' scope='%s'/>\n",
>+                              cachebank[i]->control[j].min,
>+                              cachebank[i]->control[j].reserved,
>+                              cachebank[i]->control[j].scope);
>+        }
>+        virBufferAdjustIndent(buf, -2);
>+        virBufferAddLit(buf, "</bank>\n");
>+    }
>+    virBufferAdjustIndent(buf, -2);
>+    virBufferAddLit(buf, "</cache>\n");
>+    return 0;
>+}
>+
> /**
>  * virCapabilitiesFormatXML:
>  * @caps: capabilities to format
>@@ -931,6 +982,11 @@ virCapabilitiesFormatXML(virCapsPtr caps)
>         virBufferAddLit(&buf, "</migration_features>\n");
>     }
>
>+    if (caps->host.ncachebank &&
>+            virCapabilitiesFormatCache(&buf, caps->host.ncachebank,
>+                                       caps->host.cachebank) < 0)
>+        return NULL;
>+
>     if (caps->host.netprefix)
>         virBufferAsprintf(&buf, "<netprefix>%s</netprefix>\n",
>                           caps->host.netprefix);
>diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
>index cfdc34a..b446de5 100644
>--- a/src/conf/capabilities.h
>+++ b/src/conf/capabilities.h
>@@ -138,6 +138,25 @@ struct _virCapsHostSecModel {
>     virCapsHostSecModelLabelPtr labels;
> };
>
>+typedef struct _virCapsHostCacheControl virCapsHostCacheControl;
>+typedef virCapsHostCacheControl *virCapsHostCacheControlPtr;
>+struct _virCapsHostCacheControl {
>+    unsigned long long min;
>+    unsigned long long reserved;
>+    char* scope;
>+};
>+
>+typedef struct _virCapsHostCacheBank virCapsHostCacheBank;
>+typedef virCapsHostCacheBank *virCapsHostCacheBankPtr;
>+struct _virCapsHostCacheBank {
>+    unsigned int id;
>+    char* type;
>+    char* cpus;
>+    unsigned long long size;
>+    size_t ncontrol;
>+    virCapsHostCacheControlPtr control;
>+};
>+
> typedef struct _virCapsHost virCapsHost;
> typedef virCapsHost *virCapsHostPtr;
> struct _virCapsHost {
>@@ -160,6 +179,10 @@ struct _virCapsHost {
>     size_t nsecModels;
>     virCapsHostSecModelPtr secModels;
>
>+    size_t ncachebank;
>+    size_t ncachebank_max;
>+    virCapsHostCacheBankPtr *cachebank;
>+
>     char *netprefix;
>     virCPUDefPtr cpu;
>     int nPagesSize;             /* size of pagesSize array */
>diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
>index bb7c3ad..b8445ef 100644
>--- a/src/libvirt_private.syms
>+++ b/src/libvirt_private.syms
>@@ -1125,6 +1125,7 @@ virLogManagerNew;
> # nodeinfo.h
> nodeCapsInitNUMA;
> nodeGetInfo;
>+virCapsInitCache;
> virHostCPUGetCount;
> virHostCPUGetKVMMaxVCPUs;
> virHostCPUGetMap;
>@@ -2322,8 +2323,10 @@ virRandomInt;
>
> # util/virresctrl.h
> virResCtrlAvailable;
>+virResCtrlGet;
> virResCtrlInit;
>
>+
> # util/virrotatingfile.h
> virRotatingFileReaderConsume;
> virRotatingFileReaderFree;
>diff --git a/src/nodeinfo.c b/src/nodeinfo.c
>index f2ded02..a001cd0 100644
>--- a/src/nodeinfo.c
>+++ b/src/nodeinfo.c
>@@ -48,6 +48,7 @@
> #include "virstring.h"
> #include "virnuma.h"
> #include "virlog.h"
>+#include "virresctrl.h"
>
> #define VIR_FROM_THIS VIR_FROM_NONE
>
>@@ -416,3 +417,66 @@ nodeCapsInitNUMA(virCapsPtr caps)
>     VIR_FREE(pageinfo);
>     return ret;
> }
>+
>+int
>+virCapsInitCache(virCapsPtr caps)
>+{
>+    size_t i, j;
>+    virResCtrlPtr resctrl;
>+    virCapsHostCacheBankPtr bank;
>+
>+    for (i = 0; i < VIR_RDT_RESOURCE_LAST; i++) {
>+        /* L3DATA and L3CODE share L3 resources */
>+        if (i == VIR_RDT_RESOURCE_L3CODE)
>+            continue;
>+
>+        resctrl = virResCtrlGet(i);
>+
>+        if (resctrl->enabled) {
>+            for (j = 0; j < resctrl->num_banks; j++) {
>+                if (VIR_RESIZE_N(caps->host.cachebank, caps->host.ncachebank_max,
>+                                caps->host.ncachebank, 1) < 0)
>+                    return -1;
>+
>+                if (VIR_ALLOC(bank) < 0)
>+                    return -1;
>+
>+                bank->id = resctrl->cache_banks[j].host_id;
>+                if (VIR_STRDUP(bank->type, resctrl->cache_level) < 0)
>+                    goto err;
>+                if (VIR_STRDUP(bank->cpus, virBitmapFormat(resctrl->cache_banks[j].cpu_mask)) < 0)
>+                    goto err;
>+                bank->size = resctrl->cache_banks[j].cache_size;
>+                /*L3DATA and L3CODE shares L3 cache resources, so fill them to the control element*/
>+                if (i == VIR_RDT_RESOURCE_L3DATA) {
>+                    if (VIR_EXPAND_N(bank->control, bank->ncontrol, 2) < 0)
>+                        goto err;
>+
>+                    bank->control[0].min = virResCtrlGet(VIR_RDT_RESOURCE_L3DATA)->cache_banks[j].cache_min;
>+                    bank->control[0].reserved = bank->control[0].min * (virResCtrlGet(VIR_RDT_RESOURCE_L3DATA)->min_cbm_bits);
>+                    if (VIR_STRDUP(bank->control[0].scope,
>+                                  virResCtrlGet(VIR_RDT_RESOURCE_L3DATA)->name) < 0)
>+                        goto err;
>+
>+                    bank->control[1].min = virResCtrlGet(VIR_RDT_RESOURCE_L3CODE)->cache_banks[j].cache_min;
>+                    bank->control[1].reserved = bank->control[1].min * (virResCtrlGet(VIR_RDT_RESOURCE_L3CODE)->min_cbm_bits);
>+                    if (VIR_STRDUP(bank->control[1].scope,
>+                                   virResCtrlGet(VIR_RDT_RESOURCE_L3CODE)->name) < 0)
>+                        goto err;
>+                } else {
>+                    if (VIR_EXPAND_N(bank->control, bank->ncontrol, 1) < 0)
>+                        goto err;
>+                    bank->control[0].min = resctrl->cache_banks[j].cache_min;
>+                    bank->control[0].reserved = bank->control[0].min * resctrl->min_cbm_bits;
>+                    if (VIR_STRDUP(bank->control[0].scope, resctrl->name) < 0)
>+                        goto err;
>+                }
>+                caps->host.cachebank[caps->host.ncachebank++] = bank;
>+            }
>+        }
>+    }
>+    return 0;
>+ err:
>+    VIR_FREE(bank);
>+    return -1;
>+}
>diff --git a/src/nodeinfo.h b/src/nodeinfo.h
>index 3c4dc46..5eb0f83 100644
>--- a/src/nodeinfo.h
>+++ b/src/nodeinfo.h
>@@ -28,5 +28,6 @@
>
> int nodeGetInfo(virNodeInfoPtr nodeinfo);
> int nodeCapsInitNUMA(virCapsPtr caps);
>+int virCapsInitCache(virCapsPtr caps);
>
> #endif /* __VIR_NODEINFO_H__*/
>diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
>index 359a0d8..a0d7254 100644
>--- a/src/qemu/qemu_capabilities.c
>+++ b/src/qemu/qemu_capabilities.c
>@@ -1100,6 +1100,11 @@ virQEMUCapsInitCPU(virCapsPtr caps,
>     goto cleanup;
> }
>
>+static int
>+virQEMUCapsInitCache(virCapsPtr caps)
>+{
>+    return virCapsInitCache(caps);
>+}
>
> static int
> virQEMUCapsInitPages(virCapsPtr caps)
>@@ -1146,6 +1151,9 @@ virCapsPtr virQEMUCapsInit(virQEMUCapsCachePtr cache)
>     if (virQEMUCapsInitCPU(caps, hostarch) < 0)
>         VIR_WARN("Failed to get host CPU");
>
>+    if (virQEMUCapsInitCache(caps) < 0)
>+        VIR_WARN("Failed to get host cache");
>+
>     /* Add the power management features of the host */
>     if (virNodeSuspendGetTargetMask(&caps->host.powerMgmt) < 0)
>         VIR_WARN("Failed to get host power management capabilities");
>diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>index 77d8175..3bfb4ec 100644
>--- a/src/qemu/qemu_driver.c
>+++ b/src/qemu/qemu_driver.c
>@@ -105,6 +105,7 @@
> #include "vircgroup.h"
> #include "virperf.h"
> #include "virnuma.h"
>+#include "virresctrl.h"
> #include "dirname.h"
> #include "network/bridge_driver.h"
>
>@@ -849,6 +850,9 @@ qemuStateInitialize(bool privileged,
>         run_gid = cfg->group;
>     }
>
>+    if (virResCtrlAvailable() && virResCtrlInit() < 0)
>+        VIR_WARN("Faild to initialize resource control.");
>+

s/Faild/Failed/

Also the Available() call seems unnecessary since Init() is graceful to
unavailability IIRC.

Anyway if this is to be done once per daemon lifetime, it should be
wrapped using VIR_ONCE (see VIR_ONCE_GLOBAL_INIT all over the
codebase).  I believe I mentioned this already in some previous version.

>     qemu_driver->qemuCapsCache = virQEMUCapsCacheNew(cfg->libDir,
>                                                      cfg->cacheDir,
>                                                      run_uid,
>--
>1.9.1
>
>--
>libvir-list mailing list
>libvir-list@redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH resend V10 02/12] Resctrl: expose cache information to capabilities
Posted by Eli Qiao 8 years, 3 months ago

--  
Best regards  
Eli

天涯无处不重逢
a leaf duckweed belongs to the sea, where not to meet in life  

Sent with Sparrow (http://www.sparrowmailapp.com/?sig)


On Wednesday, 15 March 2017 at 8:53 PM, Martin Kletzander wrote:

> On Mon, Mar 06, 2017 at 06:06:31PM +0800, Eli Qiao wrote:
> > This patch expose cache information to host's capabilites xml.
> >  
> > For l3 cache allocation
> > <cache>
> > <bank id='0' type='l3' size='56320' unit='KiB' cpus='0-21,44-65'>
> > <control min='2816' reserved='2816' unit='KiB' scope='L3'/>
> > </bank>
> > <bank id='1' type='l3' size='56320' unit='KiB' cpus='22-43,66-87'>
> > <control min='2816' reserved='2816' unit='KiB' scope='L3'/>
> > </bank>
> > </cache>
> >  
> > For l3 cache allocation supported cdp(seperate data/code):
> > <cache>
> > <bank id='0' type='l3' size='56320' unit='KiB' cpus='0-21,44-65'>
> > <control min='2816' reserved='2816' unit='KiB' scope='L3DATA'/>
> > <control min='2816' reserved='2816' unit='KiB' scope='L3CODE'/>
> >  
>  
>  
> I know this was discussed before, but why having a vector value in a
> single attribute? Why don't we split it to two scalars? It will also
> be SOO much more readable and close to what other tools expose, e.g.:
>  
> <control scope="data"/>
> <control scope="instruction"/>
>  
> or
>  
> <control scope="both"/>
>  
> I also skipped the 'l3' because that is already in the <bank/> and will
> never be different, right?
>  

 I am okay to ether of them, just follow with previous discussion.
  
>  
> > </bank>
> > <bank id='1' type='l3' size='56320' unit='KiB' cpus='22-43,66-87'>
> > <control min='2816' reserved='2816' unit='KiB' scope='L3DATA'/>
> > <control min='2816' reserved='2816' unit='KiB' scope='L3CODE'/>
> > </bank>
> > </cache>
> >  
> > RFC on mailing list.
> > https://www.redhat.com/archives/libvir-list/2017-January/msg00644.html
> >  
> > Signed-off-by: Eli Qiao <liyong.qiao@intel.com (mailto:liyong.qiao@intel.com)>
> > ---
> > src/conf/capabilities.c | 56 ++++++++++++++++++++++++++++++++++++++
> > src/conf/capabilities.h | 23 ++++++++++++++++
> > src/libvirt_private.syms | 3 +++
> > src/nodeinfo.c | 64 ++++++++++++++++++++++++++++++++++++++++++++
> > src/nodeinfo.h | 1 +
> > src/qemu/qemu_capabilities.c | 8 ++++++
> > src/qemu/qemu_driver.c | 4 +++
> > 7 files changed, 159 insertions(+)
> >  
> > diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c
> > index 9ab343b..23e21e4 100644
> > --- a/src/conf/capabilities.c
> > +++ b/src/conf/capabilities.c
> > @@ -198,6 +198,18 @@ virCapabilitiesClearSecModel(virCapsHostSecModelPtr secmodel)
> > }
> >  
> > static void
> > +virCapabilitiesClearCacheBank(virCapsHostCacheBankPtr cachebank)
> > +{
> > + size_t i;
> > + for (i = 0; i < cachebank->ncontrol; i++)
> > + VIR_FREE(cachebank->control[i].scope);
> > +
> > + VIR_FREE(cachebank->type);
> > + VIR_FREE(cachebank->cpus);
> > +}
> > +
> > +
> > +static void
> > virCapabilitiesDispose(void *object)
> > {
> > virCapsPtr caps = object;
> > @@ -221,6 +233,10 @@ virCapabilitiesDispose(void *object)
> > virCapabilitiesClearSecModel(&caps->host.secModels[i]);
> > VIR_FREE(caps->host.secModels);
> >  
> > + for (i = 0; i < caps->host.ncachebank; i++)
> > + virCapabilitiesClearCacheBank(caps->host.cachebank[i]);
> > + VIR_FREE(caps->host.cachebank);
> > +
> > VIR_FREE(caps->host.netprefix);
> > VIR_FREE(caps->host.pagesSize);
> > virCPUDefFree(caps->host.cpu);
> > @@ -844,6 +860,41 @@ virCapabilitiesFormatNUMATopology(virBufferPtr buf,
> > return 0;
> > }
> >  
> > +static int
> > +virCapabilitiesFormatCache(virBufferPtr buf,
> > + size_t ncachebank,
> > + virCapsHostCacheBankPtr *cachebank)
> > +{
> > + size_t i;
> > + size_t j;
> > +
> > + virBufferAddLit(buf, "<cache>\n");
> > + virBufferAdjustIndent(buf, 2);
> > +
> > + for (i = 0; i < ncachebank; i++) {
> > + virBufferAsprintf(buf,
> > + "<bank id='%u' type='%s' size='%llu' unit='KiB' cpus='%s'>\n",
> > + cachebank[i]->id,
> > + cachebank[i]->type,
> > + cachebank[i]->size,
> > + cachebank[i]->cpus);
> > +
> > + virBufferAdjustIndent(buf, 2);
> > + for (j = 0; j < cachebank[i]->ncontrol; j++) {
> > + virBufferAsprintf(buf,
> > + "<control min='%llu' reserved='%llu' unit='KiB' scope='%s'/>\n",
> > + cachebank[i]->control[j].min,
> > + cachebank[i]->control[j].reserved,
> > + cachebank[i]->control[j].scope);
> > + }
> > + virBufferAdjustIndent(buf, -2);
> > + virBufferAddLit(buf, "</bank>\n");
> > + }
> > + virBufferAdjustIndent(buf, -2);
> > + virBufferAddLit(buf, "</cache>\n");
> > + return 0;
> > +}
> > +
> > /**
> > * virCapabilitiesFormatXML:
> > * @caps: capabilities to format
> > @@ -931,6 +982,11 @@ virCapabilitiesFormatXML(virCapsPtr caps)
> > virBufferAddLit(&buf, "</migration_features>\n");
> > }
> >  
> > + if (caps->host.ncachebank &&
> > + virCapabilitiesFormatCache(&buf, caps->host.ncachebank,
> > + caps->host.cachebank) < 0)
> > + return NULL;
> > +
> > if (caps->host.netprefix)
> > virBufferAsprintf(&buf, "<netprefix>%s</netprefix>\n",
> > caps->host.netprefix);
> > diff --git a/src/conf/capabilities.h b/src/conf/capabilities.h
> > index cfdc34a..b446de5 100644
> > --- a/src/conf/capabilities.h
> > +++ b/src/conf/capabilities.h
> > @@ -138,6 +138,25 @@ struct _virCapsHostSecModel {
> > virCapsHostSecModelLabelPtr labels;
> > };
> >  
> > +typedef struct _virCapsHostCacheControl virCapsHostCacheControl;
> > +typedef virCapsHostCacheControl *virCapsHostCacheControlPtr;
> > +struct _virCapsHostCacheControl {
> > + unsigned long long min;
> > + unsigned long long reserved;
> > + char* scope;
> > +};
> > +
> > +typedef struct _virCapsHostCacheBank virCapsHostCacheBank;
> > +typedef virCapsHostCacheBank *virCapsHostCacheBankPtr;
> > +struct _virCapsHostCacheBank {
> > + unsigned int id;
> > + char* type;
> > + char* cpus;
> > + unsigned long long size;
> > + size_t ncontrol;
> > + virCapsHostCacheControlPtr control;
> > +};
> > +
> > typedef struct _virCapsHost virCapsHost;
> > typedef virCapsHost *virCapsHostPtr;
> > struct _virCapsHost {
> > @@ -160,6 +179,10 @@ struct _virCapsHost {
> > size_t nsecModels;
> > virCapsHostSecModelPtr secModels;
> >  
> > + size_t ncachebank;
> > + size_t ncachebank_max;
> > + virCapsHostCacheBankPtr *cachebank;
> > +
> > char *netprefix;
> > virCPUDefPtr cpu;
> > int nPagesSize; /* size of pagesSize array */
> > diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> > index bb7c3ad..b8445ef 100644
> > --- a/src/libvirt_private.syms
> > +++ b/src/libvirt_private.syms
> > @@ -1125,6 +1125,7 @@ virLogManagerNew;
> > # nodeinfo.h
> > nodeCapsInitNUMA;
> > nodeGetInfo;
> > +virCapsInitCache;
> > virHostCPUGetCount;
> > virHostCPUGetKVMMaxVCPUs;
> > virHostCPUGetMap;
> > @@ -2322,8 +2323,10 @@ virRandomInt;
> >  
> > # util/virresctrl.h
> > virResCtrlAvailable;
> > +virResCtrlGet;
> > virResCtrlInit;
> >  
> > +
> > # util/virrotatingfile.h
> > virRotatingFileReaderConsume;
> > virRotatingFileReaderFree;
> > diff --git a/src/nodeinfo.c b/src/nodeinfo.c
> > index f2ded02..a001cd0 100644
> > --- a/src/nodeinfo.c
> > +++ b/src/nodeinfo.c
> > @@ -48,6 +48,7 @@
> > #include "virstring.h"
> > #include "virnuma.h"
> > #include "virlog.h"
> > +#include "virresctrl.h"
> >  
> > #define VIR_FROM_THIS VIR_FROM_NONE
> >  
> > @@ -416,3 +417,66 @@ nodeCapsInitNUMA(virCapsPtr caps)
> > VIR_FREE(pageinfo);
> > return ret;
> > }
> > +
> > +int
> > +virCapsInitCache(virCapsPtr caps)
> > +{
> > + size_t i, j;
> > + virResCtrlPtr resctrl;
> > + virCapsHostCacheBankPtr bank;
> > +
> > + for (i = 0; i < VIR_RDT_RESOURCE_LAST; i++) {
> > + /* L3DATA and L3CODE share L3 resources */
> > + if (i == VIR_RDT_RESOURCE_L3CODE)
> > + continue;
> > +
> > + resctrl = virResCtrlGet(i);
> > +
> > + if (resctrl->enabled) {
> > + for (j = 0; j < resctrl->num_banks; j++) {
> > + if (VIR_RESIZE_N(caps->host.cachebank, caps->host.ncachebank_max,
> > + caps->host.ncachebank, 1) < 0)
> > + return -1;
> > +
> > + if (VIR_ALLOC(bank) < 0)
> > + return -1;
> > +
> > + bank->id = resctrl->cache_banks[j].host_id;
> > + if (VIR_STRDUP(bank->type, resctrl->cache_level) < 0)
> > + goto err;
> > + if (VIR_STRDUP(bank->cpus, virBitmapFormat(resctrl->cache_banks[j].cpu_mask)) < 0)
> > + goto err;
> > + bank->size = resctrl->cache_banks[j].cache_size;
> > + /*L3DATA and L3CODE shares L3 cache resources, so fill them to the control element*/
> > + if (i == VIR_RDT_RESOURCE_L3DATA) {
> > + if (VIR_EXPAND_N(bank->control, bank->ncontrol, 2) < 0)
> > + goto err;
> > +
> > + bank->control[0].min = virResCtrlGet(VIR_RDT_RESOURCE_L3DATA)->cache_banks[j].cache_min;
> > + bank->control[0].reserved = bank->control[0].min * (virResCtrlGet(VIR_RDT_RESOURCE_L3DATA)->min_cbm_bits);
> > + if (VIR_STRDUP(bank->control[0].scope,
> > + virResCtrlGet(VIR_RDT_RESOURCE_L3DATA)->name) < 0)
> > + goto err;
> > +
> > + bank->control[1].min = virResCtrlGet(VIR_RDT_RESOURCE_L3CODE)->cache_banks[j].cache_min;
> > + bank->control[1].reserved = bank->control[1].min * (virResCtrlGet(VIR_RDT_RESOURCE_L3CODE)->min_cbm_bits);
> > + if (VIR_STRDUP(bank->control[1].scope,
> > + virResCtrlGet(VIR_RDT_RESOURCE_L3CODE)->name) < 0)
> > + goto err;
> > + } else {
> > + if (VIR_EXPAND_N(bank->control, bank->ncontrol, 1) < 0)
> > + goto err;
> > + bank->control[0].min = resctrl->cache_banks[j].cache_min;
> > + bank->control[0].reserved = bank->control[0].min * resctrl->min_cbm_bits;
> > + if (VIR_STRDUP(bank->control[0].scope, resctrl->name) < 0)
> > + goto err;
> > + }
> > + caps->host.cachebank[caps->host.ncachebank++] = bank;
> > + }
> > + }
> > + }
> > + return 0;
> > + err:
> > + VIR_FREE(bank);
> > + return -1;
> > +}
> > diff --git a/src/nodeinfo.h b/src/nodeinfo.h
> > index 3c4dc46..5eb0f83 100644
> > --- a/src/nodeinfo.h
> > +++ b/src/nodeinfo.h
> > @@ -28,5 +28,6 @@
> >  
> > int nodeGetInfo(virNodeInfoPtr nodeinfo);
> > int nodeCapsInitNUMA(virCapsPtr caps);
> > +int virCapsInitCache(virCapsPtr caps);
> >  
> > #endif /* __VIR_NODEINFO_H__*/
> > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> > index 359a0d8..a0d7254 100644
> > --- a/src/qemu/qemu_capabilities.c
> > +++ b/src/qemu/qemu_capabilities.c
> > @@ -1100,6 +1100,11 @@ virQEMUCapsInitCPU(virCapsPtr caps,
> > goto cleanup;
> > }
> >  
> > +static int
> > +virQEMUCapsInitCache(virCapsPtr caps)
> > +{
> > + return virCapsInitCache(caps);
> > +}
> >  
> > static int
> > virQEMUCapsInitPages(virCapsPtr caps)
> > @@ -1146,6 +1151,9 @@ virCapsPtr virQEMUCapsInit(virQEMUCapsCachePtr cache)
> > if (virQEMUCapsInitCPU(caps, hostarch) < 0)
> > VIR_WARN("Failed to get host CPU");
> >  
> > + if (virQEMUCapsInitCache(caps) < 0)
> > + VIR_WARN("Failed to get host cache");
> > +
> > /* Add the power management features of the host */
> > if (virNodeSuspendGetTargetMask(&caps->host.powerMgmt) < 0)
> > VIR_WARN("Failed to get host power management capabilities");
> > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> > index 77d8175..3bfb4ec 100644
> > --- a/src/qemu/qemu_driver.c
> > +++ b/src/qemu/qemu_driver.c
> > @@ -105,6 +105,7 @@
> > #include "vircgroup.h"
> > #include "virperf.h"
> > #include "virnuma.h"
> > +#include "virresctrl.h"
> > #include "dirname.h"
> > #include "network/bridge_driver.h"
> >  
> > @@ -849,6 +850,9 @@ qemuStateInitialize(bool privileged,
> > run_gid = cfg->group;
> > }
> >  
> > + if (virResCtrlAvailable() && virResCtrlInit() < 0)
> > + VIR_WARN("Faild to initialize resource control.");
> > +
> >  
>  
>  
> s/Faild/Failed/
>  
> Also the Available() call seems unnecessary since Init() is graceful to
> unavailability IIRC.
>  
yep, we can remove  virResCtrlAvailable.

> Anyway if this is to be done once per daemon lifetime, it should be
> wrapped using VIR_ONCE (see VIR_ONCE_GLOBAL_INIT all over the
> codebase). I believe I mentioned this already in some previous version.
>  
Ops, good to know that, would like to learn how to use VIR_ONCE.  
  
> > qemu_driver->qemuCapsCache = virQEMUCapsCacheNew(cfg->libDir,
> > cfg->cacheDir,
> > run_uid,
> > --
> > 1.9.1
> >  
> > --
> > libvir-list mailing list
> > libvir-list@redhat.com (mailto:libvir-list@redhat.com)
> > https://www.redhat.com/mailman/listinfo/libvir-list
> >  
>  
>  
>  


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list