Re: [libvirt] [PATCH 15/16] tests: Add tests for caches into vircaps2xmltest

On Fri, Mar 31, 2017 at 09:56:32AM +0800, Eli Qiao wrote: > > > </cells> > Okay, cool, this comes better than my patches and have some differences. > I am open with this as long as that it can meet cache allocation requires and > everyone will be happy. > > I am ++ for this. > > But I am not sure expose all of cache information in the capabilities XML. ??? Are you not sure we "should expose" all of cache information? Or are you afraid we're not exposing enough information?

> > </topology> > > + <cache> > > + <bank id='0' level='3' type='unified' size='8192' unit='KiB' cpus='0-7'/> > > > > > eg: if enabled CDP feature on the host, what the type of level=3 cache should be like? This has nothing to do with resctrl yet. I'm just exposing the caches that exist on the host. > > + <bank id='0' level='2' type='unified' size='256' unit='KiB' cpus='0-1'/> > for the bank id, it’s per cache level unique right (data/instruction shares same id)? > It looks like it's per cache level/type unique. But it's precisely just what the kernel exposes to us. I'm not doing anything on top of that. > > + <bank id='0' level='1' type='instruction' size='32' unit='KiB' cpus='0-1'/> > > + <bank id='0' level='1' type='data' size='32' unit='KiB' cpus='0-1'/> > > + <bank id='1' level='2' type='unified' size='256' unit='KiB' cpus='2-3'/> > > + <bank id='1' level='1' type='instruction' size='32' unit='KiB' cpus='2-3'/> > > + <bank id='1' level='1' type='data' size='32' unit='KiB' cpus='2-3'/> > > + <bank id='2' level='2' type='unified' size='256' unit='KiB' cpus='4-5'/> > > + <bank id='2' level='1' type='instruction' size='32' unit='KiB' cpus='4-5'/> > > + <bank id='2' level='1' type='data' size='32' unit='KiB' cpus='4-5'/> > > + <bank id='3' level='2' type='unified' size='256' unit='KiB' cpus='6-7'/> > > + <bank id='3' level='1' type='instruction' size='32' unit='KiB' cpus='6-7'/> > > + <bank id='3' level='1' type='data' size='32' unit='KiB' cpus='6-7'/> > > + </cache> > > > > > > This’s really good that you have work this out by expose all these out to capabilities, > and it will be much easy to let resctrl keep focus on cache allocation. > > So if util/virresctrl.c would like to access some cache abilities, it will first get virCapsPtr.host.caches, > right? > Well yeah, it'll probably extend the CacheBank struct.

> but I am not sure if that’s be okay to expose all cache information which we can not > do the allocation yet. > What's the harm?

> How can a user/admin to know from capabilities? Easily. The XML you see above just says what cache is on the host. If any of the banks are allocatable, then it will have a sub-element. Is there any problem with that?

> > </host> > > > > </capabilities> > > diff --git a/tests/vircaps2xmltest.c b/tests/vircaps2xmltest.c > > index ffbe9a783811..dda0757766a8 100644 > > --- a/tests/vircaps2xmltest.c > > +++ b/tests/vircaps2xmltest.c > > @@ -58,7 +58,8 @@ test_virCapabilities(const void *opaque) > > if (!caps) > > goto cleanup; > > > > - if (virCapabilitiesInitNUMA(caps) < 0) > > + if (virCapabilitiesInitNUMA(caps) < 0 || > > + virCapabilitiesInitCaches(caps) < 0) > > goto cleanup; > > > > virSysfsSetSystemPath(NULL); > > -- > > 2.12.2 > > > > -- > > libvir-list mailing list > > libvir-list(a)redhat.com (mailto:libvir-list@redhat.com) > > https://www.redhat.com/mailman/listinfo/libvir-list > > >