[libvirt] [PATCH resend V10 02/12] Resctrl: expose cache information to capabilities
Martin Kletzander
mkletzan at redhat.com
Wed Mar 15 12:53:38 UTC 2017
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 at 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 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: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170315/877cc966/attachment-0001.sig>
More information about the libvir-list
mailing list