[libvirt] [PATCH V3] Expose resource control capabilites on cache bank

[Daniel P. Berrange]

On Fri, Apr 07, 2017 at 08:07:23PM +0800, Eli Qiao wrote:
> 
> 
> On Friday, 7 April 2017 at 7:06 PM, Daniel P. Berrange wrote:
> 
> > On Fri, Apr 07, 2017 at 01:04:57PM +0200, Martin Kletzander wrote:
> > > On Fri, Apr 07, 2017 at 05:47:54PM +0800, Eli Qiao wrote:
> > > > >  
> > > > > The name doesn't really matter that much, 'scope' makes a bit more
> > > > > sense, 'type' is consistent with the cache bank specification, I'm fine
> > > > > with any. The big question here was if it is possible to have:
> > > > >  
> > > > > <bank type='unified'>
> > > > > <control scope='code'/>
> > > > > <control scope='data'/>
> > > > > </bank>
> > > > >  
> > > > > And from what you say, the simple answer is "yes". So we need to have
> > > > > the attribute there in the control element as well.
> > > > >  
> > > >  
> > > >  
> > > >  
> > > > Dan/Martin
> > > >  
> > > > Could you please advice which should be changed ? LoL
> > > >  
> > > > This is the output if I enabled CDP
> > > >  
> > > > <cache>
> > > > <bank id='0' level='3' type='unified' size='15360' unit='KiB' cpus='0-5'>
> > > > <control min='768' unit='KiB' type='instruction' nallocations='8'/>
> > > > <control min='768' unit='KiB' type='data' nallocations='8'/>
> > > > </bank>
> > > > <bank id='1' level='3' type='unified' size='15360' unit='KiB' cpus='6-11'>
> > > > <control min='768' unit='KiB' type='instruction' nallocations='8'/>
> > > > <control min='768' unit='KiB' type='data' nallocations='8'/>
> > > > </bank>
> > > > </cache>
> > > >  
> > > >  
> > > > 1. change nallocations to allocations/max_allocation?
> > >  
> > > Dan said he's fine with both, I'd probably go for max_allocations
> > >  
> > > > 2. change type to scope ?
> > >  
> > > I don't care, pros for both in the previous mail.
> >  
> > Both attributes take the same enum values, so it is best to be consistent
> > with the attribute name.
> >  
> Thanks very much for Daniel & Martin
> 
> Forgive me to ping you again, just make sure I am in correct place :
> 
> If I read your comments correctly, we can go with:  
> 
> <cache>
>   <bank id='0' level='3' type='unified' size='15360' unit='KiB' cpus='0-5'>
>     <control min='768' unit='KiB' scope=‘code' max_allocation ='8'/>
>     <control min='768' unit='KiB' scope=‘data' max_allocation ='8'/>
>   </bank>
>   <bank id='1' level='3' type='unified' size='15360' unit='KiB' cpus='6-11'>
>     <control min='768' unit='KiB' scope=‘code' max_allocation ='8'/>
>     <control min='768' unit='KiB' scope=‘data' max_allocation ='8'/>
> 
>   </bank>
> </cache>
> 
> 
> @Daniel,  
> 
> the enum values are same with `type`
> 
> unified: 0
> instruction: 1
> data: 2
> 
> but scope should be both(0)/code(1)/data(2), so the attribute name will be

'both' and 'unified' mean the same thing.

'instruction' and 'code' mean the same thing to.

So we should use the same terminology for both attributes. IOW, I
suggest we use 'both', 'code', and 'data' everywhere.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|