-- 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 <[email protected] > > (mailto:[email protected])> > > --- > > 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 > > [email protected] (mailto:[email protected]) > > https://www.redhat.com/mailman/listinfo/libvir-list > > > > >
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
